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

Run gofumpt -w on all go files and integrate with CI #2584

Merged
merged 6 commits into from
Jul 20, 2023

Conversation

zalegrala
Copy link
Contributor

@zalegrala zalegrala commented Jun 22, 2023

What this PR does:

In working in the tempodb package, I notice a few areas that could use a little formatting. Here is a proposal to introduce a bit more opinionated formatting using gofumpt. After review of the changes, this has been integrated into make fmt to apply to the entire codebase.

If we agree on these changes, we could introduce this in tools/ and integrate as part of CI perhaps in a future PR. All editors probably have an auto-formatter as well, but worth checking on your editor. A make target could cover most of us though.

Thoughts?

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@zalegrala zalegrala changed the title Run gofumpt -w tempodb/**/*.go Run gofumpt -extra -w tempodb/**/*.go Jun 22, 2023
@mdisibio
Copy link
Contributor

The changes look fairly unopinionated and ok to me, a little bit cleaner than the default. Main concern is the tooling and preventing churn between contributors. A make target would be great.

@zalegrala
Copy link
Contributor Author

Those last few commits will do the needful. If this goes to main, make fmt will be required for all committers who don't have their editor configured to use gofumpt as their auto-format.

@zalegrala zalegrala changed the title Run gofumpt -extra -w tempodb/**/*.go Run gofumpt -extra -w on all go files and integrate with CI Jun 23, 2023
@mdisibio
Copy link
Contributor

Enabling it to format on save in VS Code was easy following the settings snippet in https://github.com/mvdan/gofumpt#visual-studio-code

However it looks like there isn't a way to include the -extra through gopls, it only has gofumpt: bool. There is a discussion of the rationale in golang/go#56403

@zalegrala
Copy link
Contributor Author

Thanks for testing @mdisibio. I'm completely open to removing the -extra flag.

@joe-elliott
Copy link
Member

i'm good on this as long as everything can be automated on save in popular editors.

we should include details about this tool and how to set it up in ./CONTRIBUTING.md.

@zalegrala zalegrala changed the title Run gofumpt -extra -w on all go files and integrate with CI Run gofumpt -w on all go files and integrate with CI Jul 17, 2023
@zalegrala zalegrala marked this pull request as ready for review July 18, 2023 17:39
@zalegrala zalegrala merged commit a5a7adb into grafana:main Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants