chore: bundle install with --with option is dispensable in this case#629
Open
sato11 wants to merge 2 commits intotwilio:mainfrom
Open
chore: bundle install with --with option is dispensable in this case#629sato11 wants to merge 2 commits intotwilio:mainfrom
bundle install with --with option is dispensable in this case#629sato11 wants to merge 2 commits intotwilio:mainfrom
Conversation
The setup script and the ci workflow definition specify that bundler should use `--with development` option, which has been deprecated as of newer version of bundler. While the intention apparently is to restrict installed gems to development group, it makes no difference to the resulting installation set. This is because we cannot rely on `--with` to opt out from installing grouped gems. What we might want to use is either `--without` option or `group :test, optional: true`. A relevant reference is here: https://bundler.io/guides/groups.html#optional-groups-and-bundlewith The discussion above in turn shows that `gem 'simplecov'` does not need to reside in test group, as it's been, and will be, installed regardless of the presence of `--with`. Therefore it's got rid of as well.
Contributor
|
Thanks for the PR! We've added the review to our backlog to be prioritised. Pull requests and +1s on the issue summary will help it move up the backlog. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
TLDR;
bundle installandbundle install --with developmentboth install the same set. So perhaps one without flag is clearer to use?The setup script and the ci workflow definition specify that bundler should use
--with developmentoption, which has been deprecated as of newer version of bundler. Mine is 2.4.3.While the intention is apparently to restrict installed gems to development group, it makes no difference to the resulting installation set.
This is because we cannot rely on
--withto opt out from installing grouped gems. What we might want to use is either--withoutoption orgroup :test, optional: true. A relevant reference is here:https://bundler.io/guides/groups.html#optional-groups-and-bundlewith
Here I propose that we drop the irrelevant flag. The discussion above in turn shows that
gem 'simplecov'does not need to reside in test group, as it's been, and will be, installed regardless of the presence of--with. Therefore it's got rid of as well.Checklist
If you have questions, please file a support ticket, or create a GitHub Issue in this repository.