-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor: adjust the order of merging kubeadm configs #2567
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
Conversation
Codecov ReportBase: 65.09% // Head: 65.09% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #2567 +/- ##
=======================================
Coverage 65.09% 65.09%
=======================================
Files 8 8
Lines 573 573
=======================================
Hits 373 373
Misses 168 168
Partials 32 32 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
76e2324
to
40b3bc2
Compare
k.ImageKubeVersion = k.getKubeVersionFromImage() | ||
for _, fn := range []string{ | ||
"", // generate default kubeadm configs | ||
k.getDefaultKubeadmConfig(), // merging from predefined path of file if file exists |
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.
- k.getDefaultKubeadmConfig()
- “”
is OK
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.
This way in the comment, as well as the original, both are setting default values first, then dst will not be overwritten even if the src is set. IMO, with mergo's transformers is more intuitive.
Signed-off-by: fengxsong <[email protected]>
Signed-off-by: fengxsong [email protected]
Seems it's already implemented but needs some adjustments. #2562