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

[CI] Add go fmt check #11440

Closed
wants to merge 1 commit into from
Closed

Conversation

djaglowski
Copy link
Member

See #11432

Our CI process is currently not ensuring that go fmt has been run.

@djaglowski djaglowski mentioned this pull request Jun 22, 2022
@djaglowski djaglowski added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 22, 2022
@mx-psi
Copy link
Member

mx-psi commented Jun 22, 2022

I believe go fmt is checked by golangci-lint

@djaglowski
Copy link
Member Author

Hmm, if I'm understanding this correctly, then make gofmt includes all *.go files regardless of build tags, whereas the linter does not.

@mx-psi
Copy link
Member

mx-psi commented Jun 22, 2022

Hmm, if I'm understanding this correctly, then make gofmt includes all *.go files regardless of build tags, whereas the linter does not.

Okay, yes, I think that's the case. Still, this doesn't feel like a problem specific to go fmt: it happens with all linters that we run as part of golangci-lint, right? The only difference is that gofmt can run on all files regardless of build tags, while other linters probably can't.

I guess this is an easy solution for go fmt, just wondering if the 'correct' solution is to run golangci-lint multiple times with different build tag combinations.

@djaglowski
Copy link
Member Author

I think you're probably right that making the change at the golangci-lint level is the more correct solution. I haven't had time to look into this further though, so I've captured it in an issue and will close this PR for now.

@djaglowski djaglowski closed this Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants