fix some linting issues and disable QF1008 (staticcheck)#3771
fix some linting issues and disable QF1008 (staticcheck)#3771thaJeztah wants to merge 4 commits intodocker:masterfrom
Conversation
thaJeztah
commented
Apr 2, 2026
thaJeztah
commented
- relates to history ls: fix '--format' flag to accept go templates #3683 (comment)
util/desktop/desktop.go:44:4: QF1012: Use fmt.Fprintf(...) instead of WriteString(fmt.Sprintf(...)) (staticcheck)
out.WriteString(fmt.Sprintf("\n %s: ", target))
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's often too opinionated; there's valid reasons to be explicit and not depend on embedding. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
requested review from
crazy-max and
jsternberg
and removed request for
crazy-max
| staticcheck: | ||
| # Enable all options, with some exceptions. | ||
| # For defaults, see https://golangci-lint.run/usage/linters/#staticcheck | ||
| checks: | ||
| - all | ||
| - -QF1008 # Omit embedded fields from selector expression; https://staticcheck.dev/docs/checks/#QF1008 |
There was a problem hiding this comment.
My bigger concern here is not even -QF1008, it's switching staticcheck from its defaults to checks: [all, -QF1008].
That is a broader policy change than the PR suggests. It doesn't just disable one noisy rule, it also opts this repo into the full staticcheck rule set instead of inheriting upstream defaults. So, we stop following staticcheck's default policy and start owning our own.
It also changes the shape of future golangci-lint bumps. By switching to all, we opt into the full staticcheck rule set, including rules that may be too opinionated or simply not a good fit for this repo, plus any new checks added upstream later. The default set is usually a more deliberate baseline across projects, whereas all pushes us toward a broader policy that may create churn, encourage questionable cleanups, or even move code away from the clearest implementation for this codebase.
I don't see enough benefit here to justify that change imo. If the actual problem is a small number of false positives or overly opinionated reports, I'd rather handle those narrowly than broaden the check set and replace the defaults entirely.
There was a problem hiding this comment.
Yeah, honestly, these settings kinda suck; the default already is all with some checks removed, but there's no option to specify default, then remove some (or add some); default is;
# Default: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]So the all will already change when updating, but yeah, it's possible they add more exclusions as a default as well 🫠
There was a problem hiding this comment.
Ah I thought default was a minimal set of rules not all of them!
There was a problem hiding this comment.
I can have a look if there's perhaps better ways to configure it. I agree that it'd be much nicer to do something like;
- default
- -SOMETHING_I_DONT_WANTDo the other changes look good to you? If so, I could probably move the config changes separate (and can look at options).
2 participants
