Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

src: lint: bump golangci-lint to 1.59, address unchecked fmt.Fprint* #12065

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jun 4, 2024

Another attempt at bumping the linter.

In 1.59, they removed a blanket Ignore: "fmt:.*", and instead now rely on a more targeted ignore set of rules based on the types that fmt.Fprint*() prints into. The main reason this bites us hard here is because when we print from urfave/cli Command runners, we use the built-in context AppWriter and ErrWriter. AppWriter is usually os.Stdout which is not excluded in the new rules, ErrWriter is usually os.Stderr but it could also be something else and the new rules can't differentiate and neither can we.

I'm fine with the more targeted ruleset. It means having to opt-in to ignores if you have good reason to. Mostly I've gone with _, _ = ignores, but there's a few places of blocks of fmt.Fprintf lines where I've gone with //nolint:errcheck.

In terms of landing this, it will have conflicts with the markets removal PRs, notably there's a big change set in paych.go which is disappearing with those PRs. This PR isn't urgent and it may be easier to rebase this on those changes once they land than the other way around.

@rvagg rvagg requested a review from rjan90 June 4, 2024 03:01
Copy link

github-actions bot commented Jun 4, 2024

All checks have completed

❌ Failed Test / Test (itest-manual_onboarding) (pull_request)

@rjan90
Copy link
Contributor

rjan90 commented Jun 4, 2024

In terms of landing this, it will have conflicts with the markets removal PRs, notably there's a big change set in paych.go which is disappearing with those PRs. This PR isn't urgent and it may be easier to rebase this on those changes once they land than the other way around.

Agree on landing this PR after the markets removal PR has landed.

@rjan90 rjan90 force-pushed the rvagg/golangci-lint@1.59 branch from 2d4863d to 8d214cd Compare June 6, 2024 09:13
@rvagg rvagg merged commit 730c96e into master Jun 6, 2024
76 of 77 checks passed
@rvagg rvagg deleted the rvagg/golangci-lint@1.59 branch June 6, 2024 09:51
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants