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

Multiple without-group support for bundler list #6939

Conversation

zaratan
Copy link

@zaratan zaratan commented Jan 31, 2019

What was the end-user problem that led to this PR?

The problem was that I needed to list all gems and dependencies that weren't in group :development or :test (or both of the together) but bundler list --without-group was only accepting one group at a time.

What was your diagnosis of the problem?

My diagnosis was I should just make it work for multiple groups

What is your fix for the problem, implemented in this PR?

My fix is I split groups by , in args.

Why did you choose this fix out of the possible options?

I chose this fix because it's simple and look like you can find in other cli.

I have two questions: Should I take care of the case where you have multiple --without-group specified ? Should I support --without-groups ?

@welcome
Copy link

welcome bot commented Jan 31, 2019

Thanks for opening a pull request and helping make Bundler better! Someone from the Bundler team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use Travis CI to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of Travis CI in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #bundler channel on Slack.

For more information about contributing to the Bundler project feel free to review our CONTRIBUTING guide

@@ -314,7 +314,7 @@ def show(gem_name = nil)
desc "list", "List all gems in the bundle"
method_option "name-only", :type => :boolean, :banner => "print only the gem names"
method_option "only-group", :type => :string, :banner => "print gems from a particular group"
method_option "without-group", :type => :string, :banner => "print all gems expect from a group"
method_option "without-group", :type => :string, :banner => "print all gems expect from 1 or multiple groups"
Copy link
Member

Choose a reason for hiding this comment

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

how does a given set of groups sound instead?

elsif @options["only-group"]
groups.select {|g| g == @options["only-group"].to_sym }
else
groups
end.map(&:to_sym)

Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove the spacing. It helps with readability.

@@ -45,13 +53,14 @@ def filtered_specs_by_groups

show_groups =
if @options["without-group"]
groups.reject {|g| g == @options["without-group"].to_sym }
without_groups.inject(groups) do |memo, without_group|
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what memo means here, can you clarify please?

@esasse
Copy link
Contributor

esasse commented May 4, 2019

@denispasin hi, are you still interested in this PR? There are pending review questions. If we don’t get an update we may have to close it. Thanks.

@deivid-rodriguez
Copy link
Member

Closing in favor of #7404. Thanks for starting this!

ghost pushed a commit that referenced this pull request Nov 4, 2019
7404: Support multiple groups for --without-group and --only-group options … r=deivid-rodriguez a=fatkodima

This is a reworked version of #6939 
It accepts multiple groups for `--without-group` and `--only-group`.

Closes #6939 

Co-authored-by: fatkodima <fatkodima123@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants