Delete unused variables and arguments#530
Conversation
|
this is the most aggressive version of this cleanup, maybe some of these are used in google3 only code? in which case i can revert those individually, or just close this and go with #529 which just marks them as |
keith
force-pushed
the
ks/delete-unused-variables-and-arguments
branch
from
7969f75 to
c09781c
Compare
ted-xie
left a comment
ted-xie
left a comment
There was a problem hiding this comment.
Mostly LGTM. I need to run some more tests internally to see if this breaks anything in unexpected ways.
|
|
||
| load("//rules:visibility.bzl", "PROJECT_VISIBILITY") | ||
| load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") | ||
| load("//rules:visibility.bzl", "PROJECT_VISIBILITY") |
There was a problem hiding this comment.
Note: load order cannot be changed here, since the internal name for bazel_skylib is sorted after the internal name for //rules:visibility.bzl.
There was a problem hiding this comment.
interesting, any idea how other places are solving this? i don't think i've ever seen an opt out for this google case and I have to imagine it was hit in other places too
There was a problem hiding this comment.
One example is rules_java: They run buildifier during the export process.
We may consider that path in the future, but not yet. Running a formatter is a non-reversible change, which makes tracing code export issues more complicated. For various reasons, rules_android's export is already super complicated, and I want to keep things simple for now.
There was a problem hiding this comment.
I pushed reverting this one but I assume there are others that hit this too?
quick check that you do set PROJECT_VISIBILITY to something else internally right?
There was a problem hiding this comment.
There have been various PRs in the past where I've manually undone load order changes.
PROJECT_VISIBILITY is called the same thing internally. It's a list that's loaded from rules/visibility.bzl. The contents of the list differ internally vs externally.
2 participants
No description provided.