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

Feature: Add parseable option to bundle config command #4919

Merged
merged 3 commits into from
Apr 15, 2017
Merged

Feature: Add parseable option to bundle config command #4919

merged 3 commits into from
Apr 15, 2017

Conversation

JuanitoFatas
Copy link
Contributor

@JuanitoFatas JuanitoFatas commented Aug 26, 2016 •
edited
Loading

This Pull Request implements #4913:

$ bundle config gem.coc --parseable
=> true

Related: #4871

Feedback for development of Bundler

Cannot use byebug / pry to dive into the code. 😢
Cannot use puts / p to print something for debugging. 😢
Currently use Bundler.ui.info to print something for debugging. 😅

@indirect
Copy link
Member

@JuanitoFatas I personally use RUBYOPT=-rpry dbundle to require pry and then run commands. Maybe we should improve the development documentation. :)

indirect
indirect reviewed Aug 26, 2016
View reviewed changes
lib/bundler/cli/config.rb
exit 1
end
if options[:parseable]
Bundler.ui.info(Bundler.settings[name])
Copy link
Member

Choose a reason for hiding this comment

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

I think probably just return here instead of putting everything inside an else block.

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 I've updated.

@JuanitoFatas JuanitoFatas mentioned this pull request Aug 26, 2016
Merged
segiddins
segiddins reviewed Aug 26, 2016
View reviewed changes
spec/commands/config_spec.rb
@@ -187,6 +187,16 @@
end
end

context "with --parseable option" do
it "returns value associated with the config" 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 add a test for when the value is unset, and also when the value is unset but bundler has a default value

@indirect indirect mentioned this pull request Aug 26, 2016
Closed
23 tasks
@indirect indirect mentioned this pull request Aug 28, 2016
Closed
homu added a commit that referenced this pull request Aug 28, 2016
@homu
Auto merge of #4921 - JuanitoFatas:doc/development-pry-byebug, r=indi…
4c4fd8e 
…rect

Doc: Add a note on how to dive into the code with Pry [ci skip]

Follow up of #4919 (comment).

[Preview DEVELOPMENT.md](https://github.com/JuanitoFatas/bundler/blob/6667a9d9a1fd7f83b4d262ff00b933f8574a974f/DEVELOPMENT.md#development-setup)
@JuanitoFatas JuanitoFatas reopened this Aug 28, 2016
homu added a commit that referenced this pull request Aug 29, 2016
@homu
Auto merge of #4921 - JuanitoFatas:doc/development-pry-byebug, r=indi…
74c359d 
…rect

Doc: Add a note on how to dive into the code with Pry [ci skip]

Follow up of #4919 (comment).

[Preview DEVELOPMENT.md](https://github.com/JuanitoFatas/bundler/blob/6667a9d9a1fd7f83b4d262ff00b933f8574a974f/DEVELOPMENT.md#development-setup)
homu added a commit that referenced this pull request Aug 29, 2016
@homu
Auto merge of #4921 - JuanitoFatas:doc/development-pry-byebug, r=indi…
fc35baa 
…rect

Doc: Add a note on how to dive into the code with Pry

Follow up of #4919 (comment).

[Preview DEVELOPMENT.md](https://github.com/JuanitoFatas/bundler/blob/6667a9d9a1fd7f83b4d262ff00b933f8574a974f/DEVELOPMENT.md#development-setup)
homu added a commit that referenced this pull request Aug 29, 2016
@homu
Auto merge of #4921 - JuanitoFatas:doc/development-pry-byebug, r=indi…
2b4b826 
…rect

Doc: Add a note on how to dive into the code with Pry

Follow up of #4919 (comment).

[Preview DEVELOPMENT.md](https://github.com/JuanitoFatas/bundler/blob/6667a9d9a1fd7f83b4d262ff00b933f8574a974f/DEVELOPMENT.md#development-setup)
segiddins
segiddins reviewed Sep 6, 2016
View reviewed changes
lib/bundler/cli/config.rb
@@ -21,6 +21,8 @@ def run
return
end

return Bundler.ui.info(Bundler.settings[name]) if options[:parseable]
Copy link
Member

Choose a reason for hiding this comment

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

What happens if parseable is passed when setting a value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean doing something like:

$ bundle config --local parseable true
$ bundle config --global parseable true

I tried, this line of change, does not affect original functionality.

Copy link
Member

Choose a reason for hiding this comment

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

is there a test for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, added in 2ba6079. The failure is fixed in #4990.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant bundle config --local --parseable foo bar

Copy link
Member

Choose a reason for hiding this comment

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

I think this is the only blocker left for merging this

@JuanitoFatas JuanitoFatas reopened this Sep 15, 2016
@indirect
Copy link
Member

@segiddins was this failure a new rails release breaking the edgecases spec?

@JuanitoFatas
Copy link
Contributor Author

@segiddins was this failure a new rails release breaking the edgecases spec?

The failure is fixed in #4990. The problem in this Pull Request is I haven't addressed #4919 (comment).

@coilysiren coilysiren mentioned this pull request Oct 9, 2016
Closed
@colby-swandale
Copy link
Member

Is this being worked on @JuanitoFatas? I have a use case for this and would like to see this feature merged possibly in 1.14.0. I'm happy to finish it off if you want.

@JuanitoFatas
Copy link
Contributor Author

I'm happy to finish it off if you want.

Please do, thank you!

@segiddins
Copy link
Member

@colby-swandale as 1-14-stable has already been branched, this won't make it in to 1.14

@colby-swandale
Copy link
Member

@segiddins thanks for letting me know. I'll still work on getting this merged in.

@colby-swandale colby-swandale mentioned this pull request Jan 5, 2017
Merged
bundlerbot added a commit that referenced this pull request Apr 15, 2017
@bundlerbot
Auto merge of #5306 - colby-swandale:bundler-config-parseable-flag, r…
7ce45d7 
…=segiddins

Bundler config parseable flag

This PR is continuing on from #4919. This adds an option to `bundle config` called `parseable` that prints the value of config without the verbose. Example:

    $ bundle config foo --parseable
    bar

Closes #4919
@bundlerbot bundlerbot merged commit 2ba6079 into rubygems:master Apr 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@segiddins segiddins segiddins left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JuanitoFatas @indirect @colby-swandale @segiddins @bundlerbot