-
Notifications
You must be signed in to change notification settings - Fork 1
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
revised how custom config is used #20
Conversation
WalkthroughThe changes involve a refinement in the configuration management system. The default value for a configuration path flag has been cleared, and there's a new approach to initializing configurations using a direct file path. This suggests a move towards more explicit configuration file handling. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- config/default/manager_auth_proxy_patch.yaml
Files selected for processing (2)
- cmd/main.go (2 hunks)
- internal/config/config.go (2 hunks)
Additional comments: 2
cmd/main.go (2)
58-64: The default value for
customConfigPath
has been changed to an empty string. Confirm that this change is intentional and ensure that it is documented, as users will need to explicitly set the path to the custom configuration file.73-82: The logic for loading the custom configuration now checks if
customConfigPath
is not empty before attempting to load. This is a good practice as it avoids unnecessary operations when no custom configuration is provided.
func InitConfig(configPath string) (*CustomConfig, error) { | ||
viper.AddConfigPath(configPath) | ||
viper.SetConfigFile(configPath) | ||
viper.SetConfigType("yaml") | ||
err := viper.ReadInConfig() | ||
if err != nil { |
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 change from viper.AddConfigPath
to viper.SetConfigFile
requires that configPath
is a full path to the configuration file. Ensure that all invocations of InitConfig
are updated to pass the full file path, not just a directory.
Verification successful
The script output confirms that customConfigPath
is used to store the path to the custom configuration file, and it is passed directly to config.InitConfig
. This aligns with the expectation that configPath
should be a full file path. There is no indication that customConfigPath
is treated as a directory path, which suggests that the change in InitConfig
to use viper.SetConfigFile
is consistent with how customConfigPath
is being set and used in cmd/main.go
.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for invocations of InitConfig to ensure they pass a full file path.
rg --type go 'InitConfig\('
Length of output: 181
Script:
#!/bin/bash
# Extract the relevant sections from cmd/main.go where customConfigPath is being set or used.
rg --type go 'customConfigPath' cmd/main.go
Length of output: 280
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.
its done
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.
Summary by CodeRabbit
Refactor
Chores