Skip to content
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

Replace Travis CI with GitHub Actions (also favor openssl/rbnacl combinations over rails compatibility tests) #381

Merged
merged 13 commits into from Dec 3, 2020

Conversation

@anakinj
Copy link
Member

@anakinj anakinj commented Oct 23, 2020

So far the CI has been checking the compatibility with different Rails versions but the risk of conflicts are slim and i'm arguing the value of the tests are minimal.

On other hand there has been some real issues with compatibility with openssl (#333), CI has always installed the latest openssl 2.x gem version. This is a little in conflict what the gem claims it's dependencies are, there is no mention that the gem requires a 2.x version of the openssl gem.

Im suggesting to drop all the tests that are involving Rails and focus on the things this gem actually uses, the openssl and signing features.

This PR will be a trial-and-error thing to test the travis configuration.

@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch from 2a5fbbc to b347c28 Oct 23, 2020
@anakinj
Copy link
Member Author

@anakinj anakinj commented Oct 23, 2020

Also this PR allows the truffleruby build to fail. It seems to be a little flaky and giving false positives. Some discussion in #368 related to that.

@anakinj
Copy link
Member Author

@anakinj anakinj commented Oct 23, 2020

And we are adding Ruby 2.7 + ruby-head builds

@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch 2 times, most recently from a24a000 to d3241fc Oct 24, 2020
Appraisals Outdated Show resolved Hide resolved
@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch 4 times, most recently from ad11255 to 88bd378 Oct 25, 2020
@anakinj
Copy link
Member Author

@anakinj anakinj commented Nov 7, 2020

@excpt What do you think about this? Think it would be valuable to ensure the compatibility with the rubies the gem support. The support for older EOL rubies could then be dropped in the next bigger release. Maybe 2.3.0?

@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch from 5d88923 to 54d36a6 Dec 1, 2020
@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch from 378be42 to 58e8b0f Dec 2, 2020
@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch from 58e8b0f to 91e17c7 Dec 2, 2020
@anakinj anakinj changed the title Suggestion on CI variations Replace Travis CI with GitHub Actions (also favor openssl/rbnacl combinations over rails compatibility tests) Dec 2, 2020
@anakinj anakinj force-pushed the anakinj:going-crazy-with-ci branch from 8b08f4e to 1698598 Dec 2, 2020
@anakinj
Copy link
Member Author

@anakinj anakinj commented Dec 2, 2020

TravisCI is really giving OSS projects a hard time. The build queue is hours long in the week.

So in all this CI tuning I converted the original change into a GitHub actions workflow, the tests just take a few minutes to execute. Also the tests can be executed in your own fork before creating a PR.

The original statement is still valid (Favor different cryptolib combinations over Rails compatibility).

An example on how the CI result would look like can be found from here

Now this also reveals that Ruby 2.3 tests without the separate openssl gem are failing because of #386

@excpt excpt self-requested a review Dec 3, 2020
@excpt
excpt approved these changes Dec 3, 2020
@excpt
Copy link
Member

@excpt excpt commented Dec 3, 2020

@anakinj Thank you very much for the work to convert the CI to github Actions.

Dropping ruby support for older versions should not be a problem.
I think we should keep an eye on the official supported list.

https://www.ruby-lang.org/en/downloads/branches/

Dropping ruby versions older than 2.4 shouldn't be a problem but this discussion should continue in a new issue.

@excpt excpt merged commit fb29072 into jwt:master Dec 3, 2020
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
codeclimate All good!
Details
@sourcelevel-bot
sourcelevel SourceLevel did not find any new or fixed issues.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants