-
Notifications
You must be signed in to change notification settings - Fork 163
Adding CustomStatus field for /info endpoint #46
Conversation
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@@ -198,6 +198,7 @@ type Info struct { | |||
Images int | |||
Driver string | |||
DriverStatus [][2]string | |||
CustomStatus [][2]string |
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.
why is it a slice of slices of two elements? it sounds like you want a map.
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.
@calavera we want the output to be displayed in the same order it was added. You're right that this is effectively a map, but using a slice of slices gives more control to the tool using CustomStatus
wrt how it will be displayed.
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.
it makes sense, can we call it CurrentStatus
or SystemStatus
? I feel like Custom
is very broad.
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.
I'll update the PR to SystemStatus
, but it would nice to have some comments that specify what it does. Is there a recommended location for that, because none of the other fields have attached comments.
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
LGTM |
LGTM. Thanks Nishant. |
Adding CustomStatus field for /info endpoint
This is for use by Docker Swarm, but generic so that other tools can use it too.
Please check related issues: moby/moby#19277 and docker-archive/classicswarm#1625