generate: Use non-null validation instead of initialization #540
Conversation
| @@ -709,7 +727,10 @@ func (g *Generator) AddLinuxResourcesHugepageLimit(pageSize string, limit uint64 | |||
|
|
|||
| // DropLinuxResourcesHugepageLimit drops a hugepage limit from g.spec.Linux.Resources.HugepageLimits. | |||
| func (g *Generator) DropLinuxResourcesHugepageLimit(pageSize string) error { | |||
| g.initSpecLinuxResources() | |||
| if g.spec == nil || g.spec.Linux == nil || g.spec.Linux.Resources == nil { | |||
There was a problem hiding this comment.
Almost every drop functions have this, I think you can add functions like initSpecLinuxResources() to form them.
There was a problem hiding this comment.
I think you can add functions like
initSpecLinuxResources()to form them.
You don't need to create ancestor properties just to unset some descendant property. If any ancestor is unset, you know the descendant is already unset. For example, currently:
$ oci-runtime-tool generate --temlate <(echo '{}') | jq .linux
null
$ oci-runtime-tool generate --template <(echo '{}') --linux-hugepage-limits-drop 2MB | jq .linux
{
"resources": {}
}With this commit we'd no longer create the useless linux.resources entry.
However, even without this commit, we are currently not clearing useless entries after we empty an array. Calling g.initSpecLinuxResources() and seeing that the entry in question is already empty, and then removing the the useless entries is a bit of busy work, but wouldn't be the end of the world. And if you decide to not bother removing the useless entries, that wouldn't be the end of the world either.
There was a problem hiding this comment.
I think you misunderstood me. I don't mean to keep initSpecLinuxResources(), I agree to remove it in Drop*() functions.
I mean to add a new functions to judge nil as init*() functions.
As we have almost same if g.spec == nil || g.spec.* == nil sentence in many place, I think we'd better add new functions to form them. Looks more elegant.
There was a problem hiding this comment.
I mean to add a new functions to judge nil...
Ah. That would work, although I don't have a problem with the inline || chain either.
There was a problem hiding this comment.
I mean to add a new functions to judge nil as init*() functions.
Fine, I'll update.
Using non-null validation makes the code more efficient. Signed-off-by: Zhou Hao <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
Using non-null validation makes the code more efficient.
Signed-off-by: Zhou Hao [email protected]