[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [win-pv-devel] [PATCH v3 5/7] Add Code Review Guide



Ian Jackson writes ("Re: [PATCH v3 5/7] Add Code Review Guide"):
> Lars Kurth writes ("Re: [PATCH v3 5/7] Add Code Review Guide"):
> > In a nutshell, in such a review the possible interpretations are
> > * I reviewed, but didn't feel qualified to do the rest
> > * I reviewed, but did not get round to give it full attention
> > * I reviewed, but before I make a final decision want to look at the next 
> > version
> > ...
> > * I reviewed and don't object the rest
> > * I reviewed and agreed with the rest 
> > The latter two are equivalent to Ack/R-b
> > 
> > That is quite a large range of possibilities and kind of leaves the author 
> > guessing what state the review is in
...
> IOW: if you do not get an A-b or R-b, then the person writing is not
> necessarily making any of the statements you posit which start "I
> reviewed".

The other point it occurs to me to make here is that whether someone
reviewed something is not of formal importance to the process,
although it is of course very relevant information.

With my maintainer hat on I frequently approve patches which I have
not reviewed.

Likewise I sometimes read and even review in detail patches where I
have no formal authority.  My comments are then intended (in a formal
sense) as input to the maintainers' decisionmaking.

(In practice, since we are all working together to make the best
software, maintainers generally expect a submitter to address issues
raised by a review from a non-maintainer, and strong objections from
non-maintainer stakeholders usually lead to a discussion about how to
resolve matters.  This is all cooperation and common courtesy I
think.)

Ian.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.