search

mvdan / gofumpt

A stricter gofmt
https://pkg.go.dev/mvdan.cc/gofumpt
BSD 3-Clause "New" or "Revised" License
3.26k stars 112 forks source link

Return another exit code when files need to be updated #114

Open myshkin5 opened 3 years ago

myshkin5 commented 3 years ago

Great tool! Thanks for creating it!

I would like to integrate gofumpt into my CI/CD pipeline so I want to know if there are changes to make. Maybe a command line option could tell gofumpt to return an error exit code if files need to be rewritten. Or this could be the default behavior with the -d option.

mvdan commented 3 years ago

I actually agree that using gofmt/gofumpt in CI/CD is a bit cumbersome, and there have been proposals to make gofmt -d return a non-zero exit status if there are differences. See https://github.com/golang/go/issues/24230.

We'll already be breaking command-line-usage compatibility with gofmt with https://github.com/mvdan/gofumpt/issues/105, so I think we can consider this feature then.

mvdan commented 3 years ago

I actually agree that using gofmt/gofumpt in CI/CD is a bit cumbersome, and there have been proposals to make gofmt -d return a non-zero exit status if there are differences. See golang/go#24230.

I ended up opening another upstream issue to make my case for it.

mvdan commented 2 years ago

Err, why did I not link it here? It's https://github.com/golang/go/issues/46289 :)

Jacalz commented 2 years ago

I think this would be a very good change. For anyone else that want to use it in their CI, I’m currently working around it using this instead: test -z $(gofumpt -d -e . | tee /dev/stderr) I hope it can help someone until this has been fixed :)

NoahHakansson commented 1 year ago

I think this would be a very good change. For anyone else that want to use it in their CI, I’m currently working around it using this instead: test -z $(gofumpt -d -e . | tee /dev/stderr) I hope it can help someone until this has been fixed :)

To get rid of the test: too many arguments add "" around it like: test -z "$(gofumpt -d -e . | tee /dev/stderr)" instead

prologic commented 1 year ago

Any chance we'll see a flag for enabling "exit status on differences"? 🤔

myshkin5 commented 1 year ago

For what it's worth, we now just use golangci-lint. It runs gofumpt in a "linter" mode (and tons of other linters too): https://golangci-lint.run/usage/linters/#gofumpt

mvdan commented 1 year ago

The chances of this feature being done in gofumpt are pretty much 100%. I just want to be careful about adding features before gofmt does, because if gofmt later adds it and in a different way (e.g. different flag name), then the result will be confusing for everyone.

I've left a comment in https://github.com/golang/go/issues/46289#issuecomment-1374508195 trying to nudge things along.

mvdan commented 2 months ago

It looks like https://github.com/golang/go/issues/46289 is finally being resolved: gofmt -d will start failing if the diff is not empty. gofumpt will be changed in exactly the same way, to ensure consistency and compatibility.