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

[Lars Kurth <lars.kurth@xxxxxxxxxx>]

On 28/11/2019, 12:12, "Rich Persaud" <persaur@xxxxxxxxx> wrote:

    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
    
That is a great idea. However, I would not want to hold up the publication of 
these documents on these suggestions. Some of them would require implementing 
tools. I was hoping there would be more progress on lore and others 
tooling/workflow related stuff by now. So I think for now, I think it is 
sufficient to set expectations better.

Regards
Lars

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