-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
libcni: up-convert a Config to a ConfigList when no other configs are found. #374
Conversation
LGTM |
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.
Some stylistic comments; apart from that lgtm
libcni/conf.go
Outdated
|
||
rawConfigList := make(map[string]interface{}) | ||
rawConfigList["name"] = original.Network.Name | ||
rawConfigList["cniVersion"] = original.Network.CNIVersion |
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.
how about
rawConfigList := map[string]interface{}{
"name" : original.Network.Name,
"cniVersion": original.Network.CNIVersion,
"plugins" : []interface{}{rawConfig},
}
libcni/conf.go
Outdated
// no files at all | ||
case len(files) == 0 && err == NoConfigsFoundError: | ||
return nil, err | ||
// some files found |
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.
could be clearer - "some files found by ConfFiles
, but not one matching the required name" ?
libcni/conf.go
Outdated
// golang default values. | ||
|
||
rawConfig := make(map[string]interface{}) | ||
err := json.Unmarshal(original.Bytes, &rawConfig) |
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.
could merge with next line
LGTM |
2719207
to
6e6ad53
Compare
@bboreham thanks for the feedback, updated. |
No description provided.