Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
v2.0.0.beta #620
v2.0.0.beta #620
Changes from 11 commits
2da1f49
e658c11
0cf7816
a593621
eb80a78
4d50d88
22192c8
fa72e43
00c212f
504b9de
e90c54b
2181127
9c88241
5ab507c
33b8bb8
3f4d395
422d762
7410730
aa52404
59c54ab
d59d53a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically speaking
Gemfile.lock
shouldn't be committed, maybe take this opportunity to remove it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the value in gitignoring Gemfile.lock in library repos? I've never been entirely clear on that. I'm guessing its that future test runs will pick up newly released versions of dependencies, giving us a heads-up if one of them fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I've always weighed this against build/failure reproducibility across development instances and found the latter more compelling, but I'd love to learn something new here. What are your views?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It prevents builds from staying in sync with reality. Your gain stability in your builds by locking to a period of time rather than versions. If you have specific versions that you need for a build to pass they should be in your
gemspec
, or if you need some custom magic in yourGemfile
.Someone who consumes the library will not get the versions specified in your
Gemfile
orGemfile.lock
no matter how they get the library, so you end up not seeing failures that affect them until someone bothers to open an issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonRowe Ah, that makes sense, and its a particularly relevant argument, considering the amount of effort this library is currently expending to address exactly that situation, i.e. tied to a 2010 reality!
I wonder though, does the existence of dependabot completely change this situation? Now that its on the job, do we get the best of both worlds by leaving Gemfile.lock checked in? Developers of the library don't suffer from works-on-my-machine, and dependabot ensures that we stay in sync with reality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependabot isn't up to that yet. It too often misses things because theres no security implication, or ignores things it can't update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, well, that's a bummer. Maybe some day. I'll remove the .lock files from git before the release.