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
feat(codegen): restore default log level, improve error messages #1211
feat(codegen): restore default log level, improve error messages #1211
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.
Lgtm
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 🥳
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.
Great! Just one comment on styling of error messages.
deployer/src/deployment/run.rs
Outdated
@@ -375,12 +377,12 @@ async fn load( | |||
if response.success { | |||
Ok(()) | |||
} else { | |||
error!(error = %response.message, "failed to load service"); | |||
error!(error = %response.message, "Failed to load service"); |
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.
Nit: IMO we should be consistent with the casing of error messages (and potentially other traces too, but at least error messages), and lowercase is the norm (e.g. rust std uses lowercase without trailing punctuation). We could also discuss this internally and agree on a standard.
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!
…huttle into feature/eng-1213-tracing-default-log-level
e8e0f12
into
shuttle-hq:feat/shuttle-logger-service
Description of change
How has this been tested? (if applicable)