gofumpt changes#488
Conversation
Looks like gofumpt is enabled on all three subprojects but evidently this diff wasn't handled prior. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
|
✅ A new PR has been created in buildah to vendor these changes: podman-container-tools/buildah#6532 |
8e0a92f to
d0867db
Compare
|
Do we have any policy on naked returns? This was the result of running gofumpt v0.9.1 locally, but there's been some back and forth between v0.9.0 and v0.9.2 apparently. |
There was a problem hiding this comment.
run gofumpt -w .
…
but there's been some back and forth between v0.9.0 and v0.9.2 apparently.
We have make fmt which uses exactly the golangci-lint version that is going to be run in CI; that should, hopefully, make it clear which version of gofumpt is relevant. (That doesn’t prevent anyone from being stricter than the golangci-lint-included version, but I think it’s unreasonable to ask PR submitters to be stricter than the automatically-ran lint.)
Do we have any policy on naked returns?
Nothing written down AFAIK. (My personal experience is that named return values, naked return, and Go’s shadowing of := within a nested block tend to result in non-obvious bugs that are avoidable by forbidding named return values; and that returning a struct with named fields scales and reads better than returning 5 values, even if named in the declaration. But that’s a personal aesthetic preference, others have different preferences and there’s a lot of code in the containers/ repo that uses named returns.)
|
I don't like naked returns as well so I am in favour of merging this even if gofumpt doesn't enforce it with the current golangci-lint version. If we all agree we don't like it maybe we should look into enabling such a check explicitly. |
|
Opening for review (and maybe merge if enough agreement). FWIW, I have neovim setup to automatically run gofumpt on every write, so that's what first caused this change when I was working on something else, so I decided to make this a separate PR. Btw, wdyt about having a unified .golangci.yml in topdir and having things like |
I… personally don’t want to spend much time tinkering, and especially bike-shedding, the best lint settings. I’m not saying there are no improvements to be had, and maybe there are classes of bugs that could be eliminated, but there are way too many outstanding bugs and RFEs for me to spend fine-tuning lint rules, or dealing with, in the worst case, dozens of formatting changes or false positives for no immediate end-user-benefit. |
See individual commits.