`bundle inject` with source and group options #5456

Shekharrajak · 2017-02-21T05:39:00Z

Eg

$ bundle inject "bootstrap" ">0" --source=https://rubygems.org --group=development
Fetching gem metadata from https://rubygems.org/............
Fetching version metadata from https://rubygems.org/..
Fetching dependency metadata from https://rubygems.org/.
Fetching gem metadata from https://rubygems.org/.............
Fetching version metadata from https://rubygems.org/..
Fetching dependency metadata from https://rubygems.org/.
Added to Gemfile:
  bootstrap (> 0), group => [:development], :source => 'https://rubygems.org'

In GemFile

gem 'bootstrap', '> 0', :group => [:development], :source => 'https://rubygems.org'

Multiple group :

$ dbundle inject "bootstrap" ">0" --source=https://rubygems.org --group=development,production
Fetching gem metadata from https://rubygems.org/............

Added to Gemfile:
gem 'bootstrap', '> 0', :group => [:development, :production], :source => 'https://rubygems.org'

In gemfile

# Added at 2017-03-24 11:40:51 +0530 by shekharrajak:
gem 'bootstrap', '> 0', :group => [:development, :production], :source => 'https://rubygems.org'

segiddins · 2017-02-21T08:59:58Z

lib/bundler/cli/inject.rb

@@ -31,7 +31,7 @@ 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 {|g| "  #{g}, group => #{g.groups.inspect}, :source => '#{g.source}'" }.join("\n")


This should probably use source.dump

~~I didn't understand what this means ? @segiddins, Can you tell me what should be replaced.~~

Edit : I have edited the line :source => '#{g.source.dump}'" . Thanks for review.

colby-swandale · 2017-02-21T09:22:09Z

spec/commands/inject_spec.rb

@@ -52,6 +52,29 @@
    end
  end

+  context "use source and group options" do


Can you please separate the source and group options into separate contexts

colby-swandale · 2017-02-21T09:25:40Z

spec/commands/inject_spec.rb

+      gemfile = bundled_app("Gemfile").read
+      str = "gem 'bootstrap', '> 0', :source => 'https://rubygems.org'"
+      expect(gemfile).to match(/str/)
+    end


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.

colby-swandale · 2017-02-21T09:30:00Z

lib/bundler/cli.rb

@@ -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 =>


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

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

Shekharrajak · 2017-02-22T07:10:02Z

I didn't get why I am getting https://travis-ci.org/bundler/bundler/jobs/203772974#L425 error in test, while it is working fine when we run this
$ dbundle inject "bootstrap" ">0" --source=https://rubygems.org --group=development in terminal.

I think this is the main error Could not reach host index.rubygems.org. Check your network connection and try again. But it must go to rubygems.org (not index.rubygems.org)

Edit : This issue is fixed (Now I am using repo source file for the testcase).

Shekharrajak · 2017-02-22T21:54:49Z

I have done the required changes and travis build passed as well. Please review it now.

Shekharrajak · 2017-02-23T11:44:28Z

Ping @colby-swandale @segiddins @indirect , please review.

colby-swandale · 2017-02-23T13:15:09Z

Please be patient, we will get around to reviewing the PR when we can.

segiddins · 2017-03-14T18:27:28Z

lib/bundler/cli/inject.rb

@@ -31,7 +31,7 @@ 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 {|g| "  #{g}, group => #{g.groups.inspect}, :source => '#{g.source.dump}'" }.join("\n")


should only include the groups and the source if they're passed in

Thanks for review. I have added new msg in this commit

segiddins · 2017-03-14T18:28:00Z

spec/commands/inject_spec.rb

+  end
+
+  context "with group option" do
+    it "add gem with group option in gemfile" do


please also add a test for including multiple groups

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

colby-swandale · 2017-03-23T22:53:56Z

ping @Shekharrajak

Shekharrajak · 2017-03-24T03:50:24Z

Sorry I completely missed the notification. I will edit the PR accordingly soon.

Shekharrajak · 2017-03-29T15:59:12Z

@colby-swandale , @segiddins Please review once when you get time . Thanks!

segiddins

👍 other than the one question

segiddins · 2017-03-29T16:03:40Z

spec/commands/inject_spec.rb

+    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]"


shouldn't this be groups instead of group?

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

Sorry for the silly mistake.

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 .

segiddins · 2017-03-30T17:40:48Z

spec/commands/inject_spec.rb

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


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

@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 ?

it can be groups as well

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

segiddins · 2017-04-05T13:12:05Z

lib/bundler/injector.rb

+        if d.groups != Array(:default)
+          group =
+            if d.groups.size == 1
+              ", :group => #{d.groups.inspect}"


If it's just one group, we shouldn't need to put it in an array

segiddins · 2017-04-07T02:06:51Z

@colby-swandale r?

colby-swandale · 2017-04-07T03:35:02Z

@bundlerbot r+

bundlerbot · 2017-04-07T03:35:03Z

📌 Commit 1754e50 has been approved by colby-swandale

bundlerbot · 2017-04-07T03:35:07Z

⌛ Testing commit 1754e50 with merge 32fb832...

…y-swandale `bundle inject` with source and group options Fixes #5452 Eg ``` $ bundle inject "bootstrap" ">0" --source=https://rubygems.org --group=development Fetching gem metadata from https://rubygems.org/............ Fetching version metadata from https://rubygems.org/.. Fetching dependency metadata from https://rubygems.org/. Fetching gem metadata from https://rubygems.org/............. Fetching version metadata from https://rubygems.org/.. Fetching dependency metadata from https://rubygems.org/. Added to Gemfile: bootstrap (> 0), group => [:development], :source => 'https://rubygems.org' ``` In GemFile ``` gem 'bootstrap', '> 0', :group => [:development], :source => 'https://rubygems.org' ``` ### Multiple group : ``` $ dbundle inject "bootstrap" ">0" --source=https://rubygems.org --group=development,production Fetching gem metadata from https://rubygems.org/............ Added to Gemfile: gem 'bootstrap', '> 0', :group => [:development, :production], :source => 'https://rubygems.org' ``` In gemfile ``` # Added at 2017-03-24 11:40:51 +0530 by shekharrajak: gem 'bootstrap', '> 0', :group => [:development, :production], :source => 'https://rubygems.org' ```

bundlerbot · 2017-04-07T04:29:40Z

☀️ Test successful - status-travis
Approved by: colby-swandale
Pushing 32fb832 to master...

bundle inject can be used with few options

069cce3

Shekharrajak mentioned this pull request Feb 21, 2017

bundle inject must use options #5452

Closed

Shekharrajak force-pushed the 5452_bundle_inject_options branch 3 times, most recently from f58089b to b7689f4 Compare February 21, 2017 07:17

test cases added

67ee37f

Shekharrajak force-pushed the 5452_bundle_inject_options branch from b7689f4 to 67ee37f Compare February 21, 2017 08:31

segiddins reviewed Feb 21, 2017

View reviewed changes

colby-swandale reviewed Feb 21, 2017

View reviewed changes

Shekharrajak force-pushed the 5452_bundle_inject_options branch from 45345b0 to 15a0208 Compare February 21, 2017 11:40

Shekharrajak force-pushed the 5452_bundle_inject_options branch 2 times, most recently from 7e2aae7 to dd31394 Compare February 22, 2017 07:12

minor changes

b78a3f0

Shekharrajak force-pushed the 5452_bundle_inject_options branch from dd31394 to b78a3f0 Compare February 22, 2017 07:14

Shekharrajak added 2 commits February 23, 2017 01:16

failed testcase fixed

9f64468

using source.dump

d73d78c

segiddins suggested changes Mar 14, 2017

View reviewed changes

multiple groups testcase added and confirmation msg added

cfe4ecc

segiddins reviewed Mar 29, 2017

View reviewed changes

minor change: typo fixed

1f466c0

segiddins reviewed Mar 30, 2017

View reviewed changes

writing groups instead of group in the Gemfile

1754e50

Shekharrajak force-pushed the 5452_bundle_inject_options branch from a4554f1 to 1754e50 Compare April 5, 2017 05:02

segiddins reviewed Apr 5, 2017

View reviewed changes

segiddins approved these changes Apr 7, 2017

View reviewed changes

colby-swandale approved these changes Apr 7, 2017

View reviewed changes

bundlerbot merged commit 1754e50 into rubygems:master Apr 7, 2017

bundlerbot mentioned this pull request Apr 7, 2017

Add command implementation #5486

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`bundle inject` with source and group options #5456

`bundle inject` with source and group options #5456

Shekharrajak commented Feb 21, 2017 •

edited

Loading

segiddins Feb 21, 2017

Shekharrajak Feb 21, 2017 •

edited

Loading

colby-swandale Feb 21, 2017

colby-swandale Feb 21, 2017 •

edited

Loading

colby-swandale Feb 21, 2017

Shekharrajak Feb 21, 2017

Shekharrajak commented Feb 22, 2017 •

edited

Loading

Shekharrajak commented Feb 22, 2017

Shekharrajak commented Feb 23, 2017

colby-swandale commented Feb 23, 2017

segiddins Mar 14, 2017

Shekharrajak Mar 24, 2017

segiddins Mar 14, 2017

Shekharrajak Mar 24, 2017

colby-swandale commented Mar 23, 2017

Shekharrajak commented Mar 24, 2017

Shekharrajak commented Mar 29, 2017

segiddins left a comment

segiddins Mar 29, 2017

Shekharrajak Mar 30, 2017

Shekharrajak Mar 30, 2017 •

edited

Loading

segiddins Mar 30, 2017

Shekharrajak Mar 31, 2017

segiddins Apr 4, 2017

Shekharrajak Apr 5, 2017

segiddins Apr 5, 2017

segiddins commented Apr 7, 2017

colby-swandale commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

bundle inject with source and group options #5456

bundle inject with source and group options #5456

Conversation

Shekharrajak commented Feb 21, 2017 • edited Loading

Multiple group :

Choose a reason for hiding this comment

Shekharrajak Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colby-swandale Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shekharrajak commented Feb 22, 2017 • edited Loading

Shekharrajak commented Feb 22, 2017

Shekharrajak commented Feb 23, 2017

colby-swandale commented Feb 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colby-swandale commented Mar 23, 2017

Shekharrajak commented Mar 24, 2017

Shekharrajak commented Mar 29, 2017

segiddins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shekharrajak Mar 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

segiddins commented Apr 7, 2017

colby-swandale commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

bundlerbot commented Apr 7, 2017

`bundle inject` with source and group options #5456

`bundle inject` with source and group options #5456

Shekharrajak commented Feb 21, 2017 •

edited

Loading

Shekharrajak Feb 21, 2017 •

edited

Loading

colby-swandale Feb 21, 2017 •

edited

Loading

Shekharrajak commented Feb 22, 2017 •

edited

Loading

Shekharrajak Mar 30, 2017 •

edited

Loading