Z dependencies installation #19
Conversation
pkg/yamlmanager/yamlmanager.go
Outdated
| // Nothing found. Poll after the period. | ||
| return false, nil | ||
| }); err != nil { | ||
| return |
There was a problem hiding this comment.
Please don't mix naked returns with regular returns
There was a problem hiding this comment.
Agreed to explicitly return the error, so that the readability is better in this case.
| func (y *YAMLManager) InstallYAML(fileName, namespace string) (objs []runtime.Object, err error) { | ||
| bb, err := ioutil.ReadFile(fileName) | ||
| if err != nil { | ||
| return |
There was a problem hiding this comment.
Maybe consider using regular returns instead of naked returns
There was a problem hiding this comment.
In this function we almost always do return objs, err. To avoid repetition, naked return is preferred.
main.go
Outdated
| setupLog.Error(err, "unable to create a new external YAML manager") | ||
| os.Exit(1) | ||
| } | ||
| objs, err := y.InstallYAML("./external.yaml", "example-partition") |
There was a problem hiding this comment.
The hard-coded string is changed to a global const partitionID, but in the run wrong, This approach is my proposal.
https://github.com/metal-stack/v?files=1
pkg/yamlmanager/yamlmanager.go
Outdated
|
|
||
| ctx := context.Background() | ||
|
|
||
| // Make sure the namespace `partitionID` exists. |
| }, nil | ||
| } | ||
|
|
||
| func (y *YAMLManager) InstallYAML(fileName, namespace string) (objs []runtime.Object, err error) { |
There was a problem hiding this comment.
fileName is camelCase, namespace is not?
Also, fileName has an implicit type?!?
There was a problem hiding this comment.
As we discussed, it should be alright to keep it this way.
No description provided.