Enable buildifier on CI

[keith]

No description provided.

[keith]

I assume this is used in google? otherwise we can toss this here instead?

[keith]

im trying to toss all the unused stuff in #530 so we can discuss that there

[keith]

it's probably possible that we need to keep some of the load() statements for google specific targets that are filtered out on export? lmk if so

[ted-xie]

Copy-pasting my comment in #530:

We may consider [running buildifier during copybara export] 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.

[keith]

Are you saying that because of that we won't want to run it externally?

[ted-xie]

Yeah, I'd want to either 1) not run buildifier on the exported git repo at all, or 2) run buildifier internally during copybara export.

Currently we're on solution 1. Solution 2 is not verboten, just not very high on my list of priorities right now, since it makes repo maintenance on the Google side more complicated.

I think actually it might make copybara import of PRs much more complicated as well.

[keith]

if load order is the only(?) major issue we can disable that rule externally in the config instead, wdyt?

[ted-xie]

I think it depends still on what kind of buildifier changes we'd see in OSS vs google3. I'm worried that they have slightly different formatting rules.

Your latest commits still seem to have load() order changes. Could you try running buildifier with load ordering disabled, like you suggested? Load order changes in WORKSPACE are fine.

The other thing I'll mention is that sometimes the big patches of empty space (for example, providers.bzl) are on purpose. Those are for code blocks that we strip out before export. That certainly complicates formatting, but IIRC it's a small set of files with the strip directives, and we could possibly just exclude those from buildifier.

[keith]

i pushed a commit without the load order changes, and disabled it in the buildifier config