Setup build matrix in CI

auth0 / omniauth-auth0

OmniAuth strategy to login with Auth0

MIT License

125 stars 67 forks source link

Setup build matrix in CI #116

Closed dmathieu closed 3 years ago

dmathieu commented 3 years ago

Changes

This configures CircleCI to use the build matrix, in order to run the tests on multiple ruby versions. The version used previously was 2.5. So I've added runs for 2.6, 2.7 and 3.0 as well.

References

There are no references for this, outside of me wanting to check that I can safely use this gem on ruby 3.0.

Testing

This isn't changing anything for users of the gem, on the contrary. It adds more tests, as we now check every supported ruby version.

Checklist

[x] I have read the Auth0 contribution guidelines
[x] I have read the Auth0 Code of Conduct
[x] All existing and new tests complete without errors
[x] All code quality tools/guidelines in the CONTRIBUTING documentation have been run/followed

davidpatrick commented 3 years ago

Hey @dmathieu thanks for contributing. Any particular reason for removing the Gemfile.lock in this PR?

dmathieu commented 3 years ago

Yes. Gemfile.lock is not accounted by gems. So it wasn't used outside the repo anyway (users of the gem could end up with a newer version that the one in the lock file). Based on that, if a specific version is required, it should be specified in the gemfile, not Gemfile.lock.

Having it also makes it harder to have the build matrix, as the bundler version will change, and the bundler packaged with 3.0 doesn't want the old bundler version specified in this Gemfile.lock.

dmathieu commented 3 years ago

As an example:

In Gemfile.lock, omniauth-oauth2 is currently set to 1.7.0. But the gemspec will allow anything in the 1.x branch.

So if I start using this gem today, I will end up on the latest version, which is 1.7.1. While this gem keeps being tester with an earlier version.

Without the Gemfile.lock, whenever a code change is made to this repo, it will always be tested against the latest supported version of the gem. If a newer version isn't compatible, the gemspec needs to prevent using it until support can be provided.

Hoping that makes sense.

davidpatrick commented 3 years ago

Hi @dmathieu thanks, yes I understand the difference of the Gemfile.lock and the gemspec, I was just curious why it was included in this PR and maybe it was by accident. Would be best to separate out those changes if we wanted to propose removing the Gemfile.lock file. I would defer to rubygems official recommendation here https://github.com/rubygems/bundler-site/pull/501/files. Basically while yes the consumer of this library doesn't receive the lock file, a contributor will, and we do not expect a new contributor to fix issues that may arise with a loosely locked gem version.

dmathieu commented 3 years ago

Sure. I've cherry-picked the first commit in this PR to #117. Because of the latest security release in omniauth, I also had to open #118.

davidpatrick commented 3 years ago

Hey @dmathieu after testing this PR, I agree with removing the Gemfile.lock. When testing across the multiple ruby versions, the lock file is getting in the way and causing complications. Since the Gemfile.lock only impacts development on this library, and has no impact on the consumers of this library, I think it is beneficial at this time to remove it. I'll close #117 and we will just move forward on this PR. Thanks for your contributions