-
Notifications
You must be signed in to change notification settings - Fork 43
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
DOCS-1827: Update Go module logging examples #2632
DOCS-1827: Update Go module logging examples #2632
Conversation
Overall readability score: 55.57 (🟢 +0)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
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.
Nice! Just some nits on CInfo
vs CInfof
vs CInfow
.
Co-authored-by: Benjamin Rewis <[email protected]>
You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/2632 |
@@ -437,7 +437,7 @@ func (b *myBase) Reconfigure(ctx context.Context, deps resource.Dependencies, co | |||
|
|||
geometries, err := kinematicbase.CollisionGeometry(conf.Frame) | |||
if err != nil { | |||
b.logger.Warnf("base %v %s", b.Name(), err.Error()) | |||
b.logger.CWarnf(ctx, "base %v %s", b.Name(), err.Error()) |
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.
Is there a reason why these methods are named with a C at the beginning? It feels rather unintuitive
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 was a decision the NetCode team made a while ago. Most loggers have methods that look like Debug
, Debugf
, and Debugw
for all log levels. Most loggers also do not take a context.Context
. We wanted to take a context on our logging methods to pass some "extra" information along with each log that could help with debugging the path a single operation was taking through the RDK. When an alternative version of a method that is otherwise identical to the original but offers a context.Context
param is written in Golang, it's common practice to name that method either FooCtx
of CFoo
. We went with the latter.
Co-authored-by: Benjamin Rewis <[email protected]>
Update Golang logging example code for module creation docs
CWarnw