Conversation
|
@poikilotherm this is new to me and probably others on the team so I'm going to post a few screenshots below to make sure it's clear what we're doing. You want me to grab the "repo token" from https://coveralls.io/github/IQSS/dataverse/settings and add it as a secret at https://github.com/IQSS/dataverse/settings/secrets/actions with the name COVERALLS_TOKEN, right? Wow, 600 stars. Squirrel! 🐿️ |
|
Yes, that's at least how I understand it. If you that token still around we might see Travis and GitHub work in parallel as long as Travis free service lasts... Passing the token is done with some auto magic on Travis, GitHub needs some luve. Thanks for looking into this. BTW you didn't star the repo yet... 🙈 |
|
@poikilotherm ok, added: |
|
As discussed on Matrix (Ha! There - we made it, @pdurbin!), we will not see a successful run for #7945. Secrets are not revealed for workflows in forked repos. Once we merge this and trigger the flow, it will be injected, as it lives inside the main repo then. I stumbled over this before, e.g. for the GITHUB_TOKEN while introducing good 🐶 @reviewdog @pdurbin and I agreed it would be OK to merge this and fix if not working as given 🎁. |
There was a problem hiding this comment.
@poikilotherm overall, this is looking really good. Thanks! Our builds on Coveralls have stopped because our builds on Travis have stopped. I'm hoping this restores them.
I'm not going to send this to QA immediately because I noticed our docs still mention Travis in several places. I'll fix that and push to your branch if I can or make a pull request into it.
I don't know if the GitHub Action will actually work or not but it's low risk so I say we just merge it (once I'm done messing with docs) and see what happens. We can always follow up with more pull requests.
I noticed we're testing on Ubuntu. I guess that's fine, especially with how CentOS has changed so much lately. We don't even deploy anything. Come to think of it, Travis probably used Ubuntu by default for all these years so my guess is there's no change from what we were doing.
One thing I'm not sure if @djbrooke and the rest of the team is very conscious of is that GitHub Actions is limited per month. I don't claim to understand the ins and outs of the billing but I've heard stuff like "2000 minutes per month for free" which seems to be supported by a table here: https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#about-billing-for-github-actions
@donsizemore has also helped us move to GitHub Actions for docs (thanks!) in pull request #7840 and @poikilotherm also helped us use it for automatic code style checking in pull request #7911. It's all good stuff but again, I don't know how the billing works.
pdurbin
left a comment
There was a problem hiding this comment.
I just updated the docs in 870389e so I'm approving this to go into QA.
As I said in a previous comment, I'm fine with just merging it and seeing if it works (if Coveralls gets updated). If it doesn't work, I'm sure we can keep working with @poikilotherm to fix it up. Thanks again!




What this PR does / why we need it:
Free service with Travis-CI.org is going away on June 15th.
This PR aims to replace unit tests and code coverage with a Github Action.
It supports caching the Maven artifacts and has initial support for a build matrix, now that JDK 17 is on the horizon.
Which issue(s) this PR closes:
Closes #7876
Special notes for your reviewer:
For Coveralls to do its thing a repo admin needs to add the Coveralls Token as a variable
COVERALLS_TOKEN.Beware that the demo in #7945 is downloading the internet (Maven...), but there is a cache action in place, which will create an initial cache once the coverage reporting works.
Suggestions on how to test this:
Watch the Github Action do its thing. It will fail to report the code coverage. See the demo PR in #7945, (direct link to run)
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.
Is there a release notes update needed for this change?:
Nope.
Additional documentation:
None.