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

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

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

There are only three possibilities:

Acked-by:
  I hereby bless this with my maintainer powers.
  I may or may not have reviewed it.  The body text may contain
  more information about that.

Reviewed-by:
  I have reviewed this to my own satisfaction and this mail contains
  all the comments I have on it.  If I am a maintainer, I hereby
  bless it with my maintainer powers.  (This is a weird use of the
  word "Reviewed" since in usual usage one can review something and
  end up disapproving of it; nevertheless this is the convention.)

In both of the above cases, additionally
  If my approval is conditional (eg on changes) this will be
  stated explicitly in the body text of my message.

Neither of the above:
  I have read at least some of this and felt motivated to make some
  observations.  If I have reviewed it properly this would usually be
  stated in the body text.  If I am taking enough of an interest in
  this patch, the body text may also say what I think the current
  state and/or next steps should be.  In any case, I do *not* bless
  this patch (in its current form) with any maintainer powers I may
  have.

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".

Having said that it is a good idea for people commenting on patches to
make clear what they have and haven't done.  I often start a message
with "Thanks for this patch which I have reviewed.  I have some
comments" or words to that effect.

Ian.

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