-
Notifications
You must be signed in to change notification settings - Fork 765
fix: ineffective viper alias #903
Conversation
Codecov Report
@@ Coverage Diff @@
## master #903 +/- ##
==========================================
+ Coverage 46.53% 46.58% +0.04%
==========================================
Files 114 114
Lines 6728 6730 +2
==========================================
+ Hits 3131 3135 +4
+ Misses 3343 3341 -2
Partials 254 254
Continue to review full report at Codecov.
|
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.
LGTM. Thanks for this quick fix.
Also let @starnop to merge this.
cmd/dfdaemon/app/root_test.go
Outdated
| r.Nil(err) | ||
| file.WriteString("supernodes:\n- 127.0.0.1:6666") | ||
| file.Close() | ||
| rootCmd.Flags().Set("config", configName) |
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.
And could you please add more test cases:
- use config file that is not exist and specify the node with cli flag
- neither use config file and cli flag to specify the node and the expected result should be
127.0.0.1:8002
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.
Currently there's no default value for supernodes. Should we add one?
|
What' s the progress? Can we move on this because we really need to fix this bug as soon as possible |
Signed-off-by: inoc603 <[email protected]>
|
LGTM. |
fix: ineffective viper alias
fix: ineffective viper alias
Signed-off-by: Jim Ma <[email protected]>
Signed-off-by: inoc603 [email protected]
Ⅰ. Describe what this PR did
Remove
viper.RegisterAlias("supernodes", "node"), and set the value by hand.Ⅱ. Does this pull request fix one issue?
fixes #900
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
Ⅳ. Describe how to verify it
create a simple
dfdaemon.yml:First use the config file only:
Look for this line in log:
Then use the
--nodeflagAnd check the cli flag overrides the config file:
Ⅴ. Special notes for reviews