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

[2.0] Implement config subcommands #5981

Merged
merged 3 commits into from
Sep 5, 2017
Merged

[2.0] Implement config subcommands #5981

merged 3 commits into from
Sep 5, 2017

Conversation

segiddins
Copy link
Member

@segiddins segiddins commented Aug 29, 2017

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

The problem was the current bundle config mega-command is hacky and confusing.

What was your diagnosis of the problem?

My diagnosis was we should add subcommands for list, get, set, and unset to make the CLI clearer.

Closes #4600.

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

My fix implements those subcommands while preserving the current bare bundle config command.

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

I chose this fix because, as opposed to #5507, we can keep the existing command in for a little while to help ease the transition.

@denniss how do you feel about this compared to your PR?

@colby-swandale
Copy link
Member

Is there any tests for this?

@segiddins
Copy link
Member Author

Not yet, need to add them if we're happy with the UX

@segiddins
Copy link
Member Author

Specs added @colby-swandale !

@segiddins segiddins changed the title Implement config subcommands [2.0] Implement config subcommands Aug 30, 2017
@segiddins segiddins added this to the 2.0 — Breaking Changes milestone Aug 31, 2017
bundle! "config get foo"
expect(last_command.stdout).to eq "Settings for `foo` in order of priority. The top value will be used\nYou have not configured a value for `foo`"

ENV["BUNDLE_FOO"] = "foo_val"
Copy link
Member

Choose a reason for hiding this comment

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

I would like to encouraging using context blocks here but i'm not super fussed right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

That just makes the specs slower though

@segiddins
Copy link
Member Author

@indirect r?

@indirect
Copy link
Member

indirect commented Sep 5, 2017

This is great, thanks! @bundlerbot r+

@bundlerbot
Copy link
Collaborator

📌 Commit 538dae8 has been approved by indirect

@bundlerbot
Copy link
Collaborator

⌛ Testing commit 538dae8 with merge 34bc135...

bundlerbot added a commit that referenced this pull request Sep 5, 2017
[2.0] Implement config subcommands

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

The problem was the current `bundle config` mega-command is hacky and confusing.

### What was your diagnosis of the problem?

My diagnosis was we should add subcommands for `list`, `get`, `set`, and `unset` to make the CLI clearer.

Closes #4600.

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

My fix implements those subcommands while preserving the current bare `bundle config` command.

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

I chose this fix because, as opposed to #5507, we can keep the existing command in for a little while to help ease the transition.

@denniss how do you feel about this compared to your PR?
@bundlerbot
Copy link
Collaborator

☀️ Test successful - status-travis
Approved by: indirect
Pushing 34bc135 to master...

@bundlerbot bundlerbot merged commit 538dae8 into master Sep 5, 2017
@segiddins segiddins deleted the seg-cleanup-config branch September 5, 2017 23:29
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.

Add subcommands to config
4 participants