[PATCH] D90352: Introduce a Bazel build configuration

Thu Jun 17 14:00:09 PDT 2021

dblaikie accepted this revision.
dblaikie added a comment.

In D90352#2825197 <https://reviews.llvm.org/D90352#2825197>, @GMNGeoffrey wrote:

> In D90352#2825057 <https://reviews.llvm.org/D90352#2825057>, @dblaikie wrote:
>
>> Might make sense to have someone from the committee that approved the proposal sign off on these files being checked into this location (with no review of the content/quality of the files) - and perhaps to commit at that point (regardless of the content/quality of the files) - then do the actual semantic review separately.
>>
>> Or if there's some part of this review that could be extracted (is there sort of a "null" implementation - maybe literally just the top level utils/bazel directory, empty, or with a README, etc) and committed with that approval, and then invested bazel-using folks could use this review thread for further discussion on the design of the bazel build files?
>
> I'm happy to restructure this patch in that way if others would prefer (check in the README first). I did think about something like that, but I thought that non-Bazel folks might want to review some of the actual substance.

They might, but I think owing to the nature of this proposal (adding files that are mostly unrelated to/meant to be ignorable by contributors) there's not going to be a lot of folks who have an opinion on the bazel file structure/contents themselves & so I doubt anyone will likely feel responsible/informed enough to sign off on their contents. You're the code owner & the code doesn't share much with the rest of the project (in terms of code style, design, etc) so there's not really much else to discuss, I think.

> I figure that the approval of the proposal already means that just the directory existing is no longer at issue, so there's not much gained by splitting it out. One thing to note is that these files have already been reviewed by at least one other LLVM community member as they've been incrementally created. Chandler wrote (ported) the bulk of them with my review and subsequent updates have all had review.
>
> It's not really clear to me what is required to land here. The proposal for adding these has been approved and I now have another community member "accepting" the patch as well as a few others commenting. It's been around for plenty of time to allow sufficient review. I think LLVM has some norms around this and for "normal" patches I think I have a good feel for what those are, but on this one I'm a little less sure :-)

Given the nature of this, I think there's not much approval required beyond the proposal approval - beyond that, it's basically yours (& anyone else who stands up to say they care deeply about bazel build files) to do with as you please.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90352/new/

https://reviews.llvm.org/D90352