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

[CLI] Dont print defaults in the command printed with --verbose#5791

Merged
bundlerbot merged 2 commits intomasterfrom
seg-verbose-cli-print-no-defaults
Jun 20, 2017
Merged

[CLI] Dont print defaults in the command printed with --verbose#5791
bundlerbot merged 2 commits intomasterfrom
seg-verbose-cli-print-no-defaults

Conversation

@segiddins
Copy link
Copy Markdown
Contributor

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

The problem was that running bundle lock --verbose would print

Running `bundle lock    --add-platform  --remove-platform  --verbose` with bundler 1.15.1

even though the platform flags were never specified. This was surfaced by #5724, which will be fixed by this PR.

Was was your diagnosis of the problem?

My diagnosis was that default values of options were being printed.

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

My fix is to filter out the default values before getting Thor

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

I chose this fix because there is no way, within the CLI instance, to tell which flags the user has explicitly given, so I felt that filtering out those options that had default values was the next-best thing.

@segiddins segiddins requested a review from colby-swandale June 20, 2017 02:14
@colby-swandale
Copy link
Copy Markdown
Member

@bundlerbot r+

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit 4614ed4 has been approved by colby-swandale

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4614ed4 with merge fb5d624...

bundlerbot added a commit that referenced this pull request Jun 20, 2017
…lby-swandale

[CLI] Dont print defaults in the command printed with --verbose

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

The problem was that running `bundle lock --verbose`  would print

    Running `bundle lock    --add-platform  --remove-platform  --verbose` with bundler 1.15.1

even though the platform flags were never specified. This was surfaced by #5724, which will be fixed by this PR.

### Was was your diagnosis of the problem?

My diagnosis was that default values of options were being printed.

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

My fix is to filter out the default values before getting Thor

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

I chose this fix because there is no way, within the `CLI` instance, to tell which flags the user has explicitly given, so I felt that filtering out those options that had default values was the next-best thing.
@bundlerbot
Copy link
Copy Markdown
Collaborator

💔 Test failed - status-travis

@segiddins
Copy link
Copy Markdown
Contributor Author

I guess we need to sort the options so the output is consistent on 1.8.7

@colby-swandale
Copy link
Copy Markdown
Member

Yea 👍 , sorry i approved this prematurely.

@segiddins
Copy link
Copy Markdown
Contributor Author

No worries! It's fine to approve stuff even if it doesn't end up passing, that's what CI is for

@segiddins
Copy link
Copy Markdown
Contributor Author

@bundlerbot r=colby-swandale

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit 531d52f has been approved by colby-swandale

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit 531d52f with merge de7d488...

bundlerbot added a commit that referenced this pull request Jun 20, 2017
…lby-swandale

[CLI] Dont print defaults in the command printed with --verbose

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

The problem was that running `bundle lock --verbose`  would print

    Running `bundle lock    --add-platform  --remove-platform  --verbose` with bundler 1.15.1

even though the platform flags were never specified. This was surfaced by #5724, which will be fixed by this PR.

### Was was your diagnosis of the problem?

My diagnosis was that default values of options were being printed.

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

My fix is to filter out the default values before getting Thor

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

I chose this fix because there is no way, within the `CLI` instance, to tell which flags the user has explicitly given, so I felt that filtering out those options that had default values was the next-best thing.
@bundlerbot
Copy link
Copy Markdown
Collaborator

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

@bundlerbot bundlerbot merged commit 531d52f into master Jun 20, 2017
@segiddins segiddins deleted the seg-verbose-cli-print-no-defaults branch June 20, 2017 20:53
@segiddins segiddins added this to the 1.15.2 milestone Jul 5, 2017
segiddins pushed a commit that referenced this pull request Jul 17, 2017
…lby-swandale

[CLI] Dont print defaults in the command printed with --verbose

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

The problem was that running `bundle lock --verbose`  would print

    Running `bundle lock    --add-platform  --remove-platform  --verbose` with bundler 1.15.1

even though the platform flags were never specified. This was surfaced by #5724, which will be fixed by this PR.

### Was was your diagnosis of the problem?

My diagnosis was that default values of options were being printed.

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

My fix is to filter out the default values before getting Thor

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

I chose this fix because there is no way, within the `CLI` instance, to tell which flags the user has explicitly given, so I felt that filtering out those options that had default values was the next-best thing.

(cherry picked from commit de7d488)

# Conflicts:
#	lib/bundler/cli.rb
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.

3 participants