-
Notifications
You must be signed in to change notification settings - Fork 61
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
[feat:] Added Config Command to Kitops CLI #523
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Akash Singh <[email protected]>
Signed-off-by: Akash Singh <[email protected]>
@amisevsk Could you please review this PR and suggest upon how we can improve it? |
Signed-off-by: Akash Singh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! I have a few initial comments
Signed-off-by: Akash Singh <[email protected]>
@amisevsk Your requested changes have been made :D |
Signed-off-by: Akash <[email protected]>
This commit refactors the code related to loading and saving the configuration file. It introduces a new function `ConfigFilePath` in the `consts.go` file to generate the path for the configuration file based on the `configHome` directory. The `LoadConfig` function now returns the default configuration if the file doesn't exist, and the `SaveConfig` function creates the directory before saving the file. These changes improve the reliability and maintainability of the configuration handling. Fixes jozu-ai#419
@amisevsk Your requested changes have been made! |
Can you add the license header for |
@gorkem license header has been added! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code that manually parses subcommands and arguments has logic errors that needs to be fixed. The code manually inspects args[0] to determine the subcommand, which is not the standard practice. It is better to Define each subcommand (set, get, list, reset) as separate Cobra commands.
Co-authored-by: Gorkem Ercan <[email protected]>
@gorkem Your requested changes have been made, could you kindly review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better but setting the logLevel did not work for me. Using reflection to find keys maybe too fragile.
…onsistent logging
@amisevsk Your requested changes have been made! |
Description
This PR introduces the new
config
command for the KitOps CLI. It allows users to manage the configuration of CLI settings more easily. With this command, users can set, get, list, and reset configuration values for persistent options like storage paths, credentials, logging levels, and other preferences.Key additions:
Config command: Introduced the
config
command with four subcommands:set
: Allows users to set configuration values.get
: Retrieves the value of a specified configuration key.list
: Lists all current configuration options and their values.reset
: Resets all configurations back to default values.Persistent root flags: The root command now uses configuration settings from the
config.json
file for default values like--log-level
,--progress
, and--verbose
, while still allowing overrides via command-line arguments.Linked Issues
Closes issue #419