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.

Sign up for GitHub

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

Jump to bottom

Setup build matrix in CI #116

Merged
merged 6 commits into from
Jan 15, 2021
Merged

Setup build matrix in CI #116

merged 6 commits into from
Jan 15, 2021

Conversation

dmathieu
Copy link
Contributor

@dmathieu dmathieu commented Jan 4, 2021 •
edited
Loading

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

@dmathieu dmathieu requested a review from a team as a code owner January 4, 2021 13:39
@davidpatrick
Copy link
Contributor

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

@dmathieu
Copy link
Contributor Author

dmathieu commented Jan 9, 2021 •
edited
Loading

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
Copy link
Contributor Author

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
Copy link
Contributor

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 dmathieu mentioned this pull request Jan 14, 2021
Closed
@dmathieu
allow GET requests in specs
da86291 
This is not something anyone should do in production. But in tests, this
allows us to bypass CSRF.
@dmathieu
remove Gemfile.lock
c81c564 
So we're not constrained on a specific bundler version.

Also, Gemfile.lock won't be used when this gem is added as a dependency
anyway. If we require a specific version, it should be set in the
gemspec file.
@dmathieu
relax bundler version
6e23ed9 
1.9 is already pretty old. There's no reason not to be able to run a
  newer version
@dmathieu
setup ruby versions matrix
f2cc126
@dmathieu
upgrade listen
7006a66 
So it runs a version supported by ruby 3.0
@dmathieu
test ruby 3.0
ba9ae24
@dmathieu
Copy link
Contributor Author

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
Copy link
Contributor

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

@davidpatrick davidpatrick merged commit 68f0d52 into auth0:master Jan 15, 2021
@davidpatrick davidpatrick added this to the 2.x milestone Jan 15, 2021
@davidpatrick davidpatrick mentioned this pull request Jan 15, 2021
Closed
4 tasks
@dmathieu dmathieu deleted the ruby-matrix branch January 15, 2021 19:42
This was referenced Jan 21, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidpatrick davidpatrick davidpatrick approved these changes

Assignees
No one assigned
Labels
CH: Changed
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dmathieu @davidpatrick