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.

Sign up for GitHub

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

Makefile: use go1.22 semantics for gofumpt #5684

Merged
merged 1 commit into from
Dec 18, 2024
+1 −1
Merged

Makefile: use go1.22 semantics for gofumpt #5684

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ shellcheck: ## run shellcheck validation
.PHONY: fmt
fmt: ## run gofumpt (if present) or gofmt
@if command -v gofumpt > /dev/null; then \
gofumpt -w -d -lang=1.21 . ; \
gofumpt -w -d -lang=1.23 . ; \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We could probably have a variable on the top of the file so it's more discoverable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Yeah, I was considering a bit how we could improve this, and went a bit back-and-forth;

  • It's not really critical (switching did not result in a diff, so looks like there were no version-specific changes)
  • It should no longer be needed when we have a go.mod, so .. 🤞 should go away in the near future (so didn't want to add the extra code-churn)
  • but I think we should look if we can containerise this (add a docker bake target); that way we would remove the requirement to have gofumpt installed, and make sure that we run it in a consistent environment (and expected version of go)
  • ☝️ similar to how we do vendoring
  • Was actually asking @crazy-max Yesterday (i.e., should we have some templates for such things?); related to Gofumpt all the things compose#12392 (comment)

else \
go list -f {{.Dir}} ./... | xargs gofmt -w -s -d ; \
fi
Expand Down
Loading