bazelbuild
Add option to sort lists by default
We work in a repo that does not care about dependency order, and we keep our lists in variables. A simple example of a typical file in our repo looks like:
DEPS = [
":a"',
":b",
":c",
]
function_call(deps=DEPS)
We'd like to keep lists sorted by default, and if we ever have a case of wanting to preserve order, we can choose to use the "do not sort" comment.
This PR adds an option to sort by lists by default, and it respects the existing "do not sort" comment.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
ℹ️ Googlers: Go here for more info.
what problem does this solve that some combination of:
tables.go and jsontables doesn't already solve in combo with # buildifier: do not sort
?
my understanding is that the code block
DEPS = [
":a"',
":b",
":c",
]
is handled by some subset of the following lines: https://github.com/bazelbuild/buildtools/blob/master/build/rewrite.go#L411-L434 These branches will only sort the list only if the list is prepended with a comment saying "keep sorted". In the current state, we would have to prepend every list with that comment, which is not ideal for us.
I'm not familiar with what jsontables is, and google doesn't seem to be returning relevant results. What is that?
It's correct, this is currently not supported.
However, I feel like this is a very niche need and it doesn't match the style recommendations in Bazel. I would encourage you to build your own program (using Buildifier as a library) to perform this transformation; you'll just need to write a few lines of Go. I don't think this feature should belong to Buildifier.
Based on https://docs.bazel.build/versions/master/skylark/build-style.html, it's true that we should Prefer to list dependencies directly, as a single list. Putting the “common” dependencies of several targets into a variable reduces maintainability, makes it impossible for tools to change the dependencies of a target and can lead to unused dependencies.
I'd push back and say that the style guide doesn't forbid the general idea of putting lists in variables, so I think there would be use for others who may keep lists in variables for whatever reason?