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

chore: introduce buildifier format/linter for starlark #18

Merged
merged 1 commit into from
Nov 18, 2023

Conversation

alexeagle
Copy link
Collaborator

So far this is just opt-in, you install from pre-commit.com and run 'pre-commit install' to get your commits formatted nicely. We can add some CI enforcement for this in a follow-up PR.

Ran 'pre-commit run --all-files' to do an initial pass over the repo and made it warning-clean.

So far this is just opt-in, you install from pre-commit.com and run 'pre-commit install' to get your commits formatted nicely.
We can add some CI enforcement for this in a follow-up PR.

Ran 'pre-commit run --all-files' to do an initial pass over the repo and made it warning-clean.
@p0deje
Copy link
Member

p0deje commented Nov 18, 2023

We can add some CI enforcement for this in a follow-up PR.

Can we have a Bazel test target to do so? This would make it much easier.

@p0deje p0deje merged commit b13ded4 into bazel-contrib:main Nov 18, 2023
15 checks passed
@alexeagle
Copy link
Collaborator Author

Can we have a Bazel test target to do so?

No, this is one of those cases where

  • we want to check the entire repo, and it's a bad antipattern to have any target that "depends on all X"
  • some of the files we want to format/lint are not part of any target - it would be annoying to be forced to create some filegroup or something that has BUILD.bazel files as its srcs

I'll send a PR now to add the https://github.com/bazel-contrib/rules-template/blob/main/.github/workflows/buildifier.yaml file

@alexeagle alexeagle deleted the buildifier branch November 18, 2023 23:11
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.

2 participants