Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

bundle inject with source and group options #5456

Merged
merged 8 commits into from
Apr 7, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/bundler/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,14 @@ def platform
end

desc "inject GEM VERSION", "Add the named gem, with version requirements, to the resolved Gemfile"
method_option "source", :type => :string, :banner =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "install gem from the given source" and "Install gem into a bundler group"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for review @colby-swandale . I have edited the PR according to to your comments.

"Install gem from the given source"
method_option "group", :type => :string, :banner =>
"Install gem into a bundler group"
def inject(name, version)
SharedHelpers.major_deprecation "The `inject` command has been replaced by the `add` command"
require "bundler/cli/inject"
Inject.new(options, name, version).run
Inject.new(options.dup, name, version).run
end

desc "lock", "Creates a lockfile without installing"
Expand Down
10 changes: 8 additions & 2 deletions lib/bundler/cli/inject.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize(options, name, version)
@options = options
@name = name
@version = version || last_version_number
@group = options[:group]
@group = options[:group].split(",") unless options[:group].nil?
@source = options[:source]
@gems = []
end
Expand All @@ -31,7 +31,13 @@ def run

if added.any?
Bundler.ui.confirm "Added to Gemfile:"
Bundler.ui.confirm added.map {|g| " #{g}" }.join("\n")
Bundler.ui.confirm(added.map do |d|
name = "'#{d.name}'"
requirement = ", '#{d.requirement}'"
group = ", :group => #{d.groups.inspect}" if d.groups != Array(:default)
source = ", :source => '#{d.source}'" unless d.source.nil?
%(gem #{name}#{requirement}#{group}#{source})
end.join("\n"))
else
Bundler.ui.confirm "All gems were already present in the Gemfile"
end
Expand Down
25 changes: 25 additions & 0 deletions spec/commands/inject_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,31 @@
end
end

context "with source option" do
it "add gem with source option in gemfile" do
bundle "inject 'foo' '>0' --source file://#{gem_repo1}"
gemfile = bundled_app("Gemfile").read
str = "gem 'foo', '> 0', :source => 'file://#{gem_repo1}'"
expect(gemfile).to include str
end
Copy link
Member

@colby-swandale colby-swandale Feb 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to use include instead if we're not using a regex with match. Example:

expect(gemfile).to include "gem 'bootstrap', '> 0', :source => 'https://rubygems.org'"

Same with the other expectations below as well.

end

context "with group option" do
it "add gem with group option in gemfile" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also add a test for including multiple groups

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for review I have added the testcase for multiple groups in this commit

bundle "inject 'rack-obama' '>0' --group=development"
gemfile = bundled_app("Gemfile").read
str = "gem 'rack-obama', '> 0', :group => [:development]"
expect(gemfile).to include str
end

it "add gem with multiple group in gemfile" do
bundle "inject 'rack-obama' '>0' --group=development,test"
gemfile = bundled_app("Gemfile").read
str = "gem 'rack-obama', '> 0', :group => [:development, :test]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be groups instead of group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @segiddins for review. I have edited the line in this commit 1f466c0

Sorry for the silly mistake.

Copy link
Contributor Author

@Shekharrajak Shekharrajak Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited this line : "add gem with multiple group in gemfile" to "add gem with multiple groups in gemfile"

and I think this line : gem 'rack-obama', '> 0', :group => [:development, :test] is correct. I followed this link : http://bundler.io/v1.3/groups.html examples .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this still reads group, and given it's the key for an array it should be groups

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@segiddins , I saw example here : http://bundler.io/v1.3/groups.html . In gemfile it must be :group => [:development, :test] right ? I haven't see groups in gemfile.

To install all dependencies, except those in specified groups
we do something :

$ bundle install --without test development

will it work when we write groups ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can be groups as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @segiddins , I have changed it in this commit 1754e50

expect(gemfile).to include str
end
end

context "when frozen" do
before do
bundle "install"
Expand Down