Skip to content

Switch to GitHub Actions and update installation instructions #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 25, 2020

Conversation

thomashoneyman
Copy link
Member

@thomashoneyman thomashoneyman commented Nov 16, 2020

This PR demonstrates switching to GitHub Actions for this repository while preserving the existing workflow from the Travis file. We can of course get fancier by caching dependencies and so on, but this presents a small enough transition that it could be applied uniformly across the core repositories. It also updates the README badge appropriately.

In addition, this updates the installation instructions to use Spago instead of Bower.

@hdgarrood
Copy link

This looks good to me but we should make sure we have a plan for Pursuit uploading after a release; that's what the after_success part in the .travis.yml file is for. I wouldn't mind saying we'll do it manually, but if we are going to switch to manual uploads then we should make sure that the whole core team is aware.

@thomashoneyman
Copy link
Member Author

I can add the after_success code back in to the action so it uploads to Pursuit with a small modification.

However, manual uploads may be the better bet; I believe with the new registry that publishing to Pursuit will be handled by the registry (cc: @f-f), and we've (unofficially) switched to manual uploads for the Contributors organization because a) the Pursuit upload doesn't trigger if you create the release via the UI and b) the Pursuit upload frequently fails unless you run the publish command twice.

All that said, if this workflow has been working pretty consistently for the core libraries then I'll gladly preserve it.

@hdgarrood
Copy link

Those are good reasons for switching to manual uploads, so I’m on board. I do think we’ve missed a few core library releases for these reasons.

@JordanMartinez
Copy link
Contributor

@thomashoneyman how hard do you think it would be to take the current contrib-updater tool and modify it so that it could update all the core repos like above? If we're going to switch to GH Actions, that's another 56 or so repos to update.

@thomashoneyman
Copy link
Member Author

@JordanMartinez I suspect we're better off with a small bash script. contrib-updater sets up the entire standard repository structure with CI, a docs folder, changelog, contributors file, issue and pr templates, possible JS linting, standardized repo labels, and so on, at least some of which requires a spago.dhall file to be in the repository.

In the case of the core libraries, all we really need to do is adjust the README installation instructions and build status badge, create the ci.yml file, and delete the travis.yml file. For that, the effort of trimming down contrib-updater may not be worth it. For example, we wouldn't even use the README template from contrib-updater.

If, one day, we do want to update the core libraries the way we updated the contrib libraries (which I hope we do!) then we should definitely repurpose contrib-updater for that. The tool just feels a bit heavyweight for this small initial change.

@hdgarrood
Copy link

Another thing to be aware of if we're wanting to automate updating the CI setup across the core libraries is that some core libraries do have tests, and for those we should first run bower install --production and attempt to build the code (to ensure that there are no devDependencies which we're using as actual dependencies), and then run bower install to install test dependencies followed by npm run -s test to run the tests. See e.g. https://github.com/purescript/purescript-arrays/blob/d93b826cccc35316e1f67b0523cf10a35812d54e/.travis.yml#L15-L19

@thomashoneyman
Copy link
Member Author

We could apply the bower install --production step to all repositories, as it doesn't break for things without devDependencies and catches things if they do have them. And I believe we can use npm run -s test --if-present to run the tests if that command exists, and not run them otherwise (though that only works for repositories which have a package.json file in the first place).

@hdgarrood
Copy link

I think the core libraries should all have package.json files, so that sounds good to me.

@thomashoneyman
Copy link
Member Author

With purescript/purescript-arrays#187 we have a demonstration of the same CI file working for both a library with tests and a library without tests.

@thomashoneyman
Copy link
Member Author

Backlinking to purescript/purescript#3962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants