-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 missing colon to errorf messages to improve readability #5911
Conversation
Tests pass locally for me, I think it's a flaky test. Cannot rerun the action - no permissions. |
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.
@@ -118,9 +118,10 @@ func main() { | |||
// If -secure_mode is not set, expose all services on -port with a regular | |||
// gRPC server. | |||
if !*secureMode { | |||
lis, err := net.Listen("tcp4", fmt.Sprintf(":%d", *port)) | |||
addr := fmt.Sprintf(":%d", *port) |
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: maybe init addr
above this block and nix line 145?
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 wanted to preserve the "laziness" of formatting, if that's ok.
I'm also seeing more errors which seems to have been formatted with the missing colon. Wondering if we should sweep the repo with a regex grep to help us locate all of them. |
That's what I did - regex, but also checked some function usage manually. I certainly may have missed some places - there is a lot of code. |
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.
Some minor nits. Thank you so much for doing this.
Thank you very much for your contribution !! |
Mostly add missing colons but also address few other minor issues.
It's confusing to read an error message without a colon for nesting, so that was the main motivation for the change. Happened to me couple of times and I couldn't find the whole string anywhere in code, wasted some time on it.
RELEASE NOTES: