[PATCH] D135531: [github] Update pip deps (NFC)

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 18 08:05:02 PDT 2022


dblaikie added a comment.

In D135531#3863837 <https://reviews.llvm.org/D135531#3863837>, @keith wrote:

> In D135531#3863681 <https://reviews.llvm.org/D135531#3863681>, @dblaikie wrote:
>
>> Looks like this and ( D135532 <https://reviews.llvm.org/D135532> ) were sent for review, but then committed before that review was attempted/completed?
>>
>> Could you please wait for reviews to be completed before submitting code.
>>
>> If code doesn't need pre-commit review (per the guidelines in the LLVM documentation), it's OK/correct to submit it without review - but the general idea is that if something is sent for review, it's because it needed it, and so submitting without that review is problematic (we don't want to encourage folks to submit stuff because they're frustrated with/waiting too long for review - if they felt it needed review in the first place, delayed code review doesn't change that fact)
>
> Yep sorry! I should have pushed these instead. For these 2 I did feel like "if someone has a herald rule for this and wants to review that's fine",

I /think/ Herald rules can also fire for direct commits, if I understand correctly?

> just because outside of the LLVM repo I generally work in a setting where everything gets review regardless of how trivial, but I get that sending for review and then landing without review sends a mixed signal, so I'll be sure to either land, or wait, in the future.

Yeah, it's a bit quirky, for sure - maybe one of these days we'll get to something more mainstream like github pull requests, etc.

We'll see - thanks for understanding!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135531



More information about the llvm-commits mailing list