refactor: container stats command #2306
Conversation
| } | ||
|
|
||
| noStream, err := cmd.Flags().GetBool("no-stream") | ||
| if err != nil { | ||
| return err | ||
| return types.ContainerStatsOptions{}, err | ||
| } | ||
|
|
||
| format, err := cmd.Flags().GetString("format") |
Signed-off-by: Ziwen Ning <ningziwe@amazon.com>
| } | ||
|
|
||
| // when (firstSet == true), we only set container stats without rendering stat entry | ||
| statsEntry, err := setContainerStatsAndRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces) |
There was a problem hiding this comment.
| statsEntry, err := setContainerStatsAndRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces) | |
| statsEntry, err := setContainerStatsThenRenderStatsEntry(previousStats, firstSet, anydata, int(task.Pid()), netNS.Interfaces) |
There was a problem hiding this comment.
"And" seems more common to me than "Then" in my impression. Any specific reasons of preferring "Then"?
(This is out of the scope of the current refactoring goal but I'm good to quickly include it if it makes sense)
There was a problem hiding this comment.
what happens behind the scenes: in the first iteration we only set ContainerStats, then in the second iteration we set a new ContainerStats and render the statistics by making diff. My idea was to reflect this strategy in the title of the function but we can keep it if you don't feel the need @AkihiroSuda @ningziwen
There was a problem hiding this comment.
To me, for the meanings, "and" and "then" should be interchangable in Go naming.
Even In daily usage, they seem very similar in most scenarios.
"I opened the door, and entered the room"
"I opened the door, then entered the room"
I'm leaning to keeping the more common one "and" but don't have strong preference.
|
I left small comment, Globally LGTM , Thanks |
#1680