Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): make config create use the same file as config root #319

Merged
merged 3 commits into from
Dec 11, 2019

Conversation

fredbi
Copy link
Contributor

@fredbi fredbi commented Dec 6, 2019

fix(cli): make config create use the same file as config root
(i.e. ~/.datamon2/datamon.yaml)

  • renamed config create source after actual command name
  • error handling
  • home dir retrieval using golang os builtin

Signed-off-by: Frederic BIDON [email protected]

@fredbi fredbi requested a review from kerneltime December 6, 2019 08:04
@fredbi fredbi force-pushed the fix-config-file-name branch from bcc2a03 to 336c8a0 Compare December 6, 2019 14:23
@fredbi fredbi requested a review from ransomw1c December 6, 2019 15:43
cmd/datamon/cmd/config.go Outdated Show resolved Hide resolved
cmd/datamon/cmd/config.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ransomw1c ransomw1c left a comment

Choose a reason for hiding this comment

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

while we're in initConfig, could the viper.AutomaticEnvironment call get moved next to where viper parses the config file if present and before values are read out of viper into the config = newConfig() object?

there's something lurking about initConfig that bothers me. encoding that the generated config file must be at the same location as the config file read by initConfig is probably part of it, but i don't think that's the entire story..

.. that datamonFlags.context.Descriptor.Name is set either by env variable or flag could be part of it, although i suspect it's a combination of things.

i could approve this as a piecemeal change as is, yet i'd like to see more done about the config story.

as usual, lmk if you'd prefer approval and an addendum merge request or discussion about how to evolve the extant work here.

cmd/datamon/cmd/config_create.go Outdated Show resolved Hide resolved
cmd/datamon/cmd/config_create.go Outdated Show resolved Hide resolved
@fredbi fredbi force-pushed the fix-config-file-name branch 2 times, most recently from 3a05c9a to 1477b6f Compare December 7, 2019 17:28
cmd/datamon/cmd/root.go Outdated Show resolved Hide resolved
@fredbi fredbi force-pushed the fix-config-file-name branch from 278f294 to a4d3b67 Compare December 10, 2019 09:37
…/.datamon2/datamon.yaml)

* renamed config create source after actual command name
* error handling
* home dir retrieval using golang os builtin

Signed-off-by: Frederic BIDON <[email protected]>

fix(cli): only tolerates yaml CLI config file

Signed-off-by: Frederic BIDON <[email protected]>
@fredbi fredbi force-pushed the fix-config-file-name branch from a4d3b67 to 9ace3d3 Compare December 10, 2019 09:54
@fredbi fredbi requested a review from ransomw1c December 10, 2019 18:21
@fredbi fredbi merged commit e183c77 into master Dec 11, 2019
@fredbi fredbi deleted the fix-config-file-name branch December 11, 2019 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants