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

Bazel: introduce buildifier formatting #16315

Merged
merged 4 commits into from
Apr 25, 2024
Merged

Bazel: introduce buildifier formatting #16315

merged 4 commits into from
Apr 25, 2024

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Apr 24, 2024

This introduces tooling and enforcement for formatting bazel files.

The tooling is provided as a bazel run target from keith/buildifier-prebuilt.

This is used in a pre-commit hook for those having that installed. In turn this is used in a CI check. Relying on a pre-commit action gives us easy checking that buildifying did not change anything in the files and printing the diff, without having to hand-roll the check ourselves.

This introduces tooling and enforcement for formatting bazel files.

The tooling is provided as a bazel run target from
[keith/buildifier-prebuilt](https://github.com/keith/buildifier-prebuilt).

This is used in a [`pre-commit`](https://pre-commit.com/) hook for those
having that installed. In turn this is used in a CI check. Relying on a
`pre-commit` action gives us easy checking that buildifying did not
change anything in the files and printing the diff, without having to
hand-roll the check ourselves.

This enforcement will make usage of gazelle easier, as gazelle itself
might reformat files, even outside of `go`. Having them properly
formatted will allow gazelle to leave them unchanged, without needing
to configure awkward exclude directives.
@redsun82
Copy link
Contributor Author

redsun82 commented Apr 24, 2024

@redsun82 redsun82 force-pushed the redsun82/buildifier branch from 6244d36 to 9def572 Compare April 24, 2024 14:36
Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

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

LGTM, good idea!

@redsun82
Copy link
Contributor Author

In the end this isn't really required for go, as I ended up restricting gazelle generation. Still think it's a good idea, so I'll go ahead and merge it 🙂

@redsun82 redsun82 merged commit 332d118 into main Apr 25, 2024
20 checks passed
@redsun82 redsun82 deleted the redsun82/buildifier branch April 25, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants