[llvm-dev] [RFC] High-Level Code-Review Documentation Update

[Renato Golin via llvm-dev]

On Mon, 18 Nov 2019 at 15:53, Finkel, Hal J. via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> I agree with this. I was planning on proposing wording along the lines of the following, adding to the original suggestion:
>
> When providing an unqualified LGTM (approval to commit), it is the responsibility of the reviewer to have reviewed all of the discussion and feedback from all reviewers ensuring that all feedback has been addressed and that all other reviewers will almost surely be satisfied with the patch being approved. If unsure, the reviewer should provide a qualified approval, (e.g., "LGTM, but please wait for @someone, @someone_else"). You may also do this if you are fairly certain that a particular community member will wish to review, even if that person hasn't done so yet (although don't wait for more than one week if that person has not responded; if you think something is "must see" by a wider audience, it should have an RFC). If it is likely that others will want to review a recently-posted patch, especially if there might be objections, but no one else has done so yet, it is also polite to provide a qualified approval (e.g., "LGTM, but please wait for a couple days in case others wish to review").
>
> Thoughts?

I felt your original proposal was missing this part, but everything
else was fine. So adding this, LGTM. :)

Maybe this paragraph is a bit convoluted and too big, but that's not
important, as long as we pass the information.

One comment on the format, though...

Not everyone reads all paragraphs, but all of them are important. I
have two tactics that make sure people pay attention to what's
necessary:

1. The first sentence of the bullet point is very short, and _conveys
the whole point_.

The paragraph can continue, expanding on the meaning, but the number
of people that will read is reduced, like 1/x for each new word. If
people understand and agree with the sentence, not much else is
needed. If they disagree or don't understand, they can go as far as
they need to make sense of it. An example is my phrase up there, and
this paragraph here. :)

2. Make the first sentence in **bold**.

This helps quick-reading and finding information, especially on long
pages/sections. Further italics or underline of specific core words
can be done, if necessary. Honestly, I could have said "short and
bold" on the first bullet point, but I wanted to have two, to
demonstrate the point, as the first sentece was already too long and
people won't read the word "bold", even if it was in bold.

cheers,
--renato