[llvm-dev] Policy question about Phabricator reviews

[Paul C. Anagnostopoulos via llvm-dev]

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?