Re: [MirageOS-devel] [Xen-devel] [PATCH v2 4/6] Add Code Review Guide

[Rich Persaud <persaur@xxxxxxxxx>]

On Nov 28, 2019, at 05:12, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
> On 28.11.2019 01:54, Stefano Stabellini wrote:
>>> On Thu, 26 Sep 2019, Lars Kurth wrote:
>>> From: Lars Kurth <lars.kurth@xxxxxxxxxx>
>>> 
>>> This document highlights what reviewers such as maintainers and committers 
>>> look
>>> for when reviewing code. It sets expectations for code authors and provides
>>> a framework for code reviewers.
>> 
>> I think the document is missing a couple of things:
>> 
>> - a simple one line statement that possibly the most important thing in
>>  a code review is to indentify any bugs in the code
>> 
>> - an explanation that requests for major changes to the series should be
>>  made early on (i.e. let's not change the architecture of a feature at
>>  v9 if possible) I also made this comment in reply to patch #5. I'll
>>  let you decide where is the best place for it.
> 
> This needs balancing. People crucial to the evaluation of a new
> feature and its implementation simply may not have the time to
> reply prior to v9. We've had situations where people posted new
> revisions every other day, sometimes even more than one per day.
> 
> As indicated in several other contexts before - imo people not
> helping to shoulder the review load should also not have the
> expectation that their (large) contributions will be looked at
> in due course. 

To make this actionable, we could have:

- reviewer demand index:  automated index of open patches still in need of 
review, sorted by decreasing age

- review flow control:  each new patch submission cites one recent review by 
the patch submitter, for a patch of comparable size

- reviewer supply growth:  a bootstrapping guide for new reviewers and 
submitters, with patterns, anti-patterns, and examples to be emulated

Rich
_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/mirageos-devel