fix(cli): switch automatic version check based on config#5290
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5290 +/- ##
==========================================
- Coverage 53.21% 52.74% -0.47%
==========================================
Files 304 298 -6
Lines 17793 17528 -265
Branches 813 813
==========================================
- Hits 9468 9245 -223
+ Misses 7444 7410 -34
+ Partials 881 873 -8
|
vadasambar
force-pushed
the
fix/5218/fix-automatic-version-check-flag
branch
2 times, most recently
from
2c0cbc3 to
c6a3f74
Compare
vadasambar
changed the title
cli/cmd/root.go
Outdated
There was a problem hiding this comment.
I have used CLIConfigManager here instead of viper because we already have it in place and it works well.
TODO for the future can be to re-write NewCLIConfigManager with viper.
cli/cmd/root_test.go
Outdated
There was a problem hiding this comment.
Doing this because changing rootCLIConfig directly can have impact on other tests.
cli/cmd/root.go
Outdated
There was a problem hiding this comment.
Passing rootCLIConfig just makes it easier to test runVersionCheck
- use cfg mgr instead of viper to load config -- we already use it in `set config` - pass cliconfig as an argument to runVersionCheck -- makes it easier to test - add hook function to shallow copy cli config -- this is so that cli config can be tweaked for the tests - remove viper from root cmd (not used anymore) Signed-off-by: Suraj Banakar (vadasambar) <[email protected]>
vadasambar
force-pushed
the
fix/5218/fix-automatic-version-check-flag
branch
from
c6a3f74 to
77c31da
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| } | ||
|
|
||
| viper.AutomaticEnv() // read in environment variables that match | ||
| logging.PrintLog(fmt.Sprintf("Using config file: %s", cfgMgr.CLIConfigPath), logging.InfoLevel) |
There was a problem hiding this comment.
I think we can remove this log output. Other than that the PR looks good :)
There was a problem hiding this comment.
Printing the above log message is the default behavior. Do you want me to change this? If yes, I would like to know the reason. Thank you!
There was a problem hiding this comment.
The main reason I was thinking of is that it might interfere with pipelines where the keptn CLI is used within scripts - but I just checked that the --quiet flag is applied correctly here, meaning that the message is not shown if this flag is set. So therefore we can leave it in there
2 participants
This PR
Fixes #5218
How to test
You should not be able to reproduce the issue (check under "To Reproduce") in this comment