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. _______________________________________________ flashrom mailing list -- flashrom@flashrom.org To unsubscribe send an email to flashrom-le...@flashrom.org
