Skip to content

Rename configuration file to cobra-cli.yaml#6

Open
marckhouzam wants to merge 1 commit into
spf13:mainfrom
VilledeMontreal:feat/configFile
Open

Rename configuration file to cobra-cli.yaml#6
marckhouzam wants to merge 1 commit into
spf13:mainfrom
VilledeMontreal:feat/configFile

Conversation

@marckhouzam
Copy link
Copy Markdown
Collaborator

During the discussion about creating the cobra-cli repo @spf13 hinted that we should rename the configuration file to cobra-cli.yaml but keep supporting the cobra.yaml name as a fallback. Please see spf13/cobra#1597 (comment)

This PR does this. If a different approach is preferred, just let me know.

/cc @liggitt @jpmcb

Copy link
Copy Markdown
Collaborator

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Makes sense!

@johnSchnake
Copy link
Copy Markdown
Collaborator

LGTM to me. Since its a behavior change on before a release I'll defer the merging to the (already) tagged folks.

Comment thread cmd/root.go Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread cmd/root.go
}

func initConfig() {
viper.AutomaticEnv()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is there a reason this was moved above config file loading? I'm not familiar with what this does, but if possible, I'd like to keep this PR scoped to just the config file changes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The call to viper.AutomaticEnv() was originally done before the call to viper.ReadInConfig() and since I moved the latter to within the if/else logic, I moved the viper.AutomaticEnv() higher up to avoid duplicating it. But I can keep it closer to viper.ReadInConfig() by coding it twice.

@marckhouzam
Copy link
Copy Markdown
Collaborator Author

Thanks @liggitt for realizing that I missed the dot-prefix for all the config file references!

With the renaming of the generator to cobra-cli, this commit also
renames the configuration file as ".cobra-cli.yaml".  For backwards-
compatibility, if "$HOME/.cobra-cli.yaml" can't be read, we fallback
to "$HOME/.cobra.yaml".

Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
@marckhouzam
Copy link
Copy Markdown
Collaborator Author

Should we go ahead and merge this since the v1.3.0 release is done?

@marckhouzam marckhouzam added this to the v1.3.1 milestone Mar 17, 2022
@github-actions
Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@umarcor
Copy link
Copy Markdown
Contributor

umarcor commented May 17, 2022

@marckhouzam cmd/root.go was modified in 9e07cb5. Do you mind rebasing this PR?

@github-actions
Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions
Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions
Copy link
Copy Markdown

This PR is being marked as stale due to a long period of inactivity

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 2, 2023

This PR is being marked as stale due to a long period of inactivity

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2023

This PR is being marked as stale due to a long period of inactivity

@jpmcb
Copy link
Copy Markdown
Collaborator

jpmcb commented Jun 23, 2023

@marckhouzam - if you're feeling good with this one, let's go ahead and merge it 👀

@marckhouzam
Copy link
Copy Markdown
Collaborator Author

Thanks @jpmcb. I'll rebase then merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants