Skip to content
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

Closed
wants to merge 11 commits into from
Closed

chore: use revive linter #5712

wants to merge 11 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Aug 6, 2023

This PR enables the revive linter, and should be merged after gci and gofumpt, since it contains them.

  • use gofmput to format code
  • use gci to format imports
  • reconfigure gci
  • linter autofixes
  • rearrange imports a little
  • all changes other than unused-parameter
  • use revive linter

Contains

@faddat faddat mentioned this pull request Aug 6, 2023
modules/caddyhttp/ip_range.go Outdated Show resolved Hide resolved
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens otherwise?

Copy link
Contributor Author

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.

modules/caddyhttp/push/handler.go Show resolved Hide resolved
Comment on lines 772 to +773
var best []*Upstream
var bestReqs int = -1
bestReqs := -1
Copy link
Member

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.

modules/caddyhttp/fileserver/matcher.go Outdated Show resolved Hide resolved
modules/logging/nopencoder.go Outdated Show resolved Hide resolved
@faddat faddat mentioned this pull request Aug 6, 2023
@francislavoie
Copy link
Member

I'll review again after the parent PRs are merged, hard to follow the diffs until then.

@francislavoie francislavoie changed the title use revive linter chore: use revive linter Aug 14, 2023
@faddat faddat marked this pull request as draft August 14, 2023 05:36
@faddat
Copy link
Contributor Author

faddat commented Aug 14, 2023

Sorry, this one should be on draft for a minute. I may have revive configured a little too spicy.

@francislavoie -- perfect <3

@faddat
Copy link
Contributor Author

faddat commented Aug 14, 2023

I think this PR is no longer needed.

Prefer #5709

@faddat faddat closed this Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants