gnoi icon indicating copy to clipboard operation
gnoi copied to clipboard

Clarify default reboot method

Open greg-dennis opened this issue 3 years ago • 2 comments

The documentation on Reboot includes the phrase "Only the DEFAULT method ..." https://github.com/openconfig/gnoi/blob/master/system/system.proto#L101

But there is no RebootMethod named "DEFAULT". There is one named "UNKNOWN" but the documentation next to it says that it is "invalid": https://github.com/openconfig/gnoi/blob/master/system/system.proto#L124

I don't know if we are above renaming proto fields, but I'd consider renaming "UNKNOWN" to "DEFAULT" and removing "Invalid" from the documentation description. Also, should probably fix the spelling of "after" at #L101

greg-dennis avatar Apr 09 '21 05:04 greg-dennis

It looks like the intention was to actually make COLD be the only method (with delay ) that is required to be supported. So we can probably leave UNKNOWN as the standard zero value and update the comment to say that only COLD is guaranteed to be supported, and should also perhaps be the default if method is not specified. In any case, agree the comments could use some cleanup/clarification.

aashaikh avatar Apr 09 '21 06:04 aashaikh

Could it also clarify whether specifying the RebootMethod is required and the behavior if it is not specified?

greg-dennis avatar Apr 09 '21 12:04 greg-dennis