-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
chore: use revive linter #5712
chore: use revive linter #5712
Conversation
@@ -630,7 +630,7 @@ func AdminAPIRequest(adminAddr, method, uri string, headers http.Header, body io | |||
if err != nil { | |||
return nil, fmt.Errorf("making request: %v", err) | |||
} | |||
if parsedAddr.IsUnixNetwork() { | |||
if parsedAddr.IsUnixNetwork() { //nolint:revive |
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.
What happens otherwise?
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.
the revive linter prompts us to remove the empty code block that is currently just comments.
var best []*Upstream | ||
var bestReqs int = -1 | ||
bestReqs := -1 |
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.
Nitpick but this is actually less symmetrical so it looks less nice. But whatever, it's fine.
I'll review again after the parent PRs are merged, hard to follow the diffs until then. |
Sorry, this one should be on draft for a minute. I may have revive configured a little too spicy. @francislavoie -- perfect <3 |
I think this PR is no longer needed. Prefer #5709 |
This PR enables the revive linter, and should be merged after gci and gofumpt, since it contains them.
Contains