-
Notifications
You must be signed in to change notification settings - Fork 424
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
Add a buildifier warning to enforce rule load location #1318
base: main
Are you sure you want to change the base?
Conversation
c1806b6
to
7f228ee
Compare
@vladmos could you take a look, please? |
Is it actually recommended anywhere that Also, please fix the merge conflict. |
6011c0b
to
283c8f9
Compare
Updated to use "//tools/bazel:genrule.bzl" instead. This should promote better practices to structure folders with Bazel libraries. |
@vladmos could you take another look, please? |
I'm sorry, I still don't understand where the recommendation come from. If that's been discussed e.g. in a mailing list, could you please attach a link? I can imagine that some smaller projects have their own code structure and enforcing a fixed load location without apparent upsides can be noisy. Besides, isn't "genrule" a built-in function that doesn't need to be loaded at all? |
There is no such recommendation and it wasn't discussed. There is an issue that describes the problem: #1231 The location of rules is organization-specific and depends on how source code is organized in a given organization. I don't want to provide any recommendations and this PR is not about that. However I'd like to avoid bad examples, so I changed the example location to avoid promoting a practice of keeping bazel rules at the root of a repository. I'd rather avoid discussing recommendations of any kids in this PR. The purpose of this change it give users a way to enforce their custom rules. |
c6d01e9
to
a319d34
Compare
Addresses #1231, but without auto fixing.