[llvm-dev] Policy question about Phabricator reviews

[Stefan Stipanovic via llvm-dev]

I think you should, but if you are not sure, you can always ask for
clarification in the review.

On Sun, Aug 16, 2020 at 4:54 PM Paul C. Anagnostopoulos <paul at windfall.com>
wrote:

> At 8/16/2020 10:39 AM, Stefan Stipanovic wrote:
> >Hi Paul,
> >
> >I've read "LLVM Code-Review Policies and Practices," but I remain unsure
> of a couple of things. Do I always wait for an actual "LGTM", or can people
> approve the patch for submission in other ways?
> >
> >The patch is accepted through Phabricator with or without a message (the
> message is usually LGTM or something similar)
>
> Aha! I didn't see the green Accepted indicator in the upper-left. I should
> look around more carefully.
>
> ---------------------------------------------
>
>
> >Â What happens when a patch is approved but then there are additional
> review comments? Should the patch be submitted as is, then a follow-up
> patch submitted, or should the additional review comments be incorporated
> first?
> >
> >I think this depends on the patch. In most cases I'd incorporate the
> changes first and if changes are not nits, probably wait for another
> approval. Usually it is mentioned if you need to address comments in a
> follow-up.
>
> It's a new document and the post-acceptance review is reasonably
> substantial AND by a different reviewer. I should have made that clear in
> my first post. Do you think I should incorporate the other reviewer's
> comments first?
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200816/bd9f2da3/attachment.html>