Skip to content
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

Clarify default reboot method #50

Open
greg-dennis opened this issue Apr 9, 2021 · 2 comments
Open

Clarify default reboot method #50

greg-dennis opened this issue Apr 9, 2021 · 2 comments

Comments

@greg-dennis
Copy link
Contributor

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

@aashaikh
Copy link
Contributor

aashaikh commented Apr 9, 2021

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.

@greg-dennis
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants