-
Notifications
You must be signed in to change notification settings - Fork 244
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
Add documentation for logging in odo and odo create enhancements #673
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.
Please remove any cli-reference.md changes. We auto generate this through: https://github.com/redhat-developer/odo/blob/master/scripts/cli-reference/generate-cli-reference.go if you want to make changes, you should look at the cmd
folder and edit the help menu / descriptions there!
docs/development.md
Outdated
@@ -122,7 +122,7 @@ When new git tag is created, Travis-ci deploy job automatically builds binaries | |||
Do not upload any binaries for release | |||
When new tag is created Travis-CI starts a special deploy job. | |||
This job builds binaries automatically (via `make prepare-release`) and then uploads it to GitHub release page (done using odo-bot user). | |||
3. When job fishes you should see binaries on GitHub release page. Release is now marked as a draft. Update descriptions and publish release. | |||
3. When job finishes you should see binaries on GitHub release page. Release is now marked as a draft. Update descriptions and publish release. |
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.
When a job finishes
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.
on the GitHub release page
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.
Ack
docs/logging.md
Outdated
@@ -0,0 +1,41 @@ | |||
# Logging in ODO | |||
|
|||
[Glog](https://godoc.org/github.com/golang/glog) is used for v style logging in ODO. |
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.
ODO should be Odo
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.
V style
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.
ack
docs/logging.md
Outdated
# Logging in ODO | ||
|
||
[Glog](https://godoc.org/github.com/golang/glog) is used for v style logging in ODO. | ||
glog provides functions such as Info, Warning, Error, Fatal, plus formatting variants such as Infof. It also provides V-style logging controlled by the -v for better runtime control |
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 don't we need this in here. Perhaps we can replace it with the different levels of logging we use? For example, we do not yet know how many levels in-terms of numbers. (zero to 4?)
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.
0 to 9 as in the link in https://github.com/redhat-developer/odo/pull/673/files#diff-6fede6782d33a3306a99cb1a47ea5f6bR28
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.
added the same here now
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
This PR adds the following information: 1. Enhancements in odo create command added to docs/cli-reference.md 2. docs/logging.md describes about the use of glog for logging fixes redhat-developer#667 Signed-off-by: anmolbabu <anmolbudugutta@gmail.com>
@@ -122,6 +122,10 @@ func init() { | |||
pflag.CommandLine.AddGoFlagSet(flag.CommandLine) | |||
pflag.CommandLine.Set("logtostderr", "true") | |||
|
|||
// Override the verbosity flag description | |||
verbosity := pflag.Lookup("v") | |||
verbosity.Usage += ". Level varies from 0 to 9 (default 0)." |
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.
👍
docs/logging.md
Outdated
|
||
## Working | ||
|
||
Every ODO command takes an optional flag `-v` that must be used with an integer log level in the range from 0-9. And any INFO severity log statement that is logged at a level lesser than or equal to the one passed with the command alongside `-v` flag will be logged to the STDOUT. |
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.
Should be Odo
not ODO.
Any INFO severity log statement
Don't start sentences with "And"
logged to STDOUT
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.
Done
docs/logging.md
Outdated
|
||
Every ODO command takes an optional flag `-v` that must be used with an integer log level in the range from 0-9. And any INFO severity log statement that is logged at a level lesser than or equal to the one passed with the command alongside `-v` flag will be logged to the STDOUT. | ||
|
||
All ERROR severity level log messages will always be logged immaterial of the passed `v` level. |
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.
always be logged regardless of the passed V level.
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.
Done
docs/logging.md
Outdated
|
||
Every source file that needs to log messages will need to import glog as under: | ||
|
||
``` |
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.
make it ```go
not ```
so there is formatting in markdown
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.
Did you mean get ``` and the statement in same line ?
If so, done
docs/logging.md
Outdated
|
||
## Usage | ||
|
||
Every source file that needs to log messages will need to import glog as under: |
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.
Every source file that requires logging will need to import glog
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.
Done
docs/logging.md
Outdated
glog.V(4).Infof(msg, args...) | ||
``` | ||
|
||
For more info level logging conventions please refer [here](https://kubernetes.io/docs/reference/kubectl/cheatsheet/#kubectl-output-verbosity-and-debugging) |
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.
missing period at end of sentence
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.
Done
docs/logging.md
Outdated
``` | ||
glog.Warningf(msg, args...) | ||
``` | ||
Similarly there are also Info, Error and Warning counterparts of each of thes above that don't take format args but instead a string message |
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.
missing newline above
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.
Similarly there is also
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.
these
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.
Actually, maybe remove this sentence. The user can look at https://kubernetes.io/docs/reference/kubectl/cheatsheet/#kubectl-output-verbosity-and-debugging if they need help
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.
Done
Signed-off-by: anmolbabu <anmolbudugutta@gmail.com>
This LGTM! Let's merge. |
This PR adds the following information:
fixes #667
Signed-off-by: anmolbabu anmolbudugutta@gmail.com