Code Reviews guidelines discussion

[Anastasia Klimchuk]

Hello People,

Few weeks ago at the flashrom meeting we had a discussion about code
reviews and the situations that sometimes happen. We decided that it
would be good to have some guidelines… and then we decided that I will
start writing something. I have been thinking about it since then. I
even started writing something, but I wasn’t happy at all: it felt
like composing a wall of text that no one would be reading.

I decided to think about: what problems are we solving? And then it
got much better. I gathered the list of problems that I think are
present and repeatedly happen during code reviews. All of the below is
RFC and of course, if there is a problem that is missing, please tell.

I would really appreciate everyone’s opinions on this!

What I suggest is: after agreeing on the list of problems, compose and
agree the answers to them (yes easier to say then to do :)), and then
it can be like “Code Reviews FAQ” or something like that? Especially
if we can encourage people to read the other point of view first.

—--------------------------------------
## Contributor point of view ##
—--------------------------------------

#1 Is this comment a conversation or an action item?

#2 Is this comment blocking patch from merging or non-blocking?

#3 Are we waiting for anyone [else] to have a look at this patch?

#4 Is my patch ready to be merged? / Why is my patch not being merged?

#5 Who are the best reviewers for this patch?

#6 This patch is a real emergency, what is the process for that?

—-----------------------------------------------------
## Reviewer / Maintainer point of view ##
—-----------------------------------------------------

#1 My important comment(s) got ignored and the patch was merged regardless.

#2 I have so many code reviews, and everyone wants it fast and
complains reviews are too slow.

#3 A patch that was merged broke something, but people don't want to revert.

#4 I asked the patch owner to fix something, but they are arguing with
me instead, and wasting everyone's time.

#5 What this patch is doing, and why is it needed?

#6 I want to review the patch, but I am busy for the next X weeks.
-- 
Anastasia.