-
Notifications
You must be signed in to change notification settings - Fork 104
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
WIP - Updating name and documentation for Transparent HelloWorld Service #189
base: master
Are you sure you want to change the base?
Conversation
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.
tr-helloworld looks more aesthetic than trhelloworld IMO.
Let's try; I remember something like the "-" character creates troubles with the code generation :-( |
Yes looks swagger auto-gem compiler has problem to generate correct code if there is "-" character, the character following the "-" will be used to generate variable but some place capital some place small-case and code won't be compiled. |
@@ -23,7 +23,7 @@ module transparenthelloworld { | |||
enum SLOWPATH; | |||
} | |||
default PASS; | |||
description "Action performed on ingress packets"; | |||
description "Action performed on ingress packets (DROP/PASS/SLOWPATH; default: PASS)"; |
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 think information about possible values and default should not be part of the description.
Maybe the help generation could be improved to include this.
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.
Right point, Mauricio. Moved now in the "example" section of the YANG. However, the example is not printed on screen by the help, so we should probably modify the command line to include that as well.
Few comments:
|
6edf5aa
to
72d0300
Compare
Unfortunately this is not true. Long names are consuming a lot of space on the screen. |
I soft agree with that, just find a name that is shorter but still meaningful, I think |
Signed-off-by: Fulvio Risso <[email protected]>
Signed-off-by: Fulvio Risso <[email protected]>
72d0300
to
b8eff81
Compare
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.
LGTM
Added documentation, useful for new developers.
It would be nice to shorten the name, so that we can avoid typing "transparenthelloworld" such as in
polycubectl transparenthelloworld add trhw0
but I'm unable to do so right now.