Skip to content

Add default config command#67

Open
hahmed wants to merge 5 commits into
gel-rb:mainfrom
hahmed:add-default-config-command
Open

Add default config command#67
hahmed wants to merge 5 commits into
gel-rb:mainfrom
hahmed:add-default-config-command

Conversation

@hahmed

@hahmed hahmed commented May 15, 2019

Copy link
Copy Markdown
Contributor

Adding a default config command so we can use gel like so;

gel config

Which will output all the default config settings, e.g.

jobs=3
gem.test=minitest
gem.mit=true

@hahmed hahmed changed the title Add default config command May 15, 2019
@hahmed hahmed changed the title [WIP] Add default config command May 17, 2019
@hahmed hahmed marked this pull request as ready for review May 17, 2019 23:37
Comment thread test/fixtures/config Outdated
@@ -0,0 +1,4 @@
jobs:
: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops... (while jobs isn't a valid config setting anyway,) this isn't supposed to look like that.

I think this:

group, key = key.split(".", 2)

needs to be followed by something like:

group, key = nil, group if key.nil?

On the other hand, maybe we should change the file format (or group/key format?) to something different. The current behaviour looks not-great when key is a DNS name, as #73's example shows for packages.shopify.io. But that's a thought for another day.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With bundle config local.GEM_NAME /path/to/local/git/repository and bundle config jobs 5 we get the output:

.bundle % cat config
---
BUNDLE_GEM__TEST: "minitest"
BUNDLE_JOBS: "7"
BUNDLE_LOCAL__GEM_NAME: "/path/to/local/git/repository"

To be honest bundlers output is not great either - WDYT? I'm going to give this a little more thought.

Comment thread lib/gel/config.rb
end
key, value = line.split(':')
next unless key
result[key.downcase] = value&.strip

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@matthewd I downcased to keep all settings consistent, also strip the value too. Without the null check & some tests fail, I think it's because somewhere we are using the default -- bundler version for the config where we run all our tests, is that how it's supposed to work?

@hahmed hahmed requested a review from matthewd October 7, 2019 21:13
Base automatically changed from master to main January 16, 2021 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants