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

[Julien Grall <julien@xxxxxxx>]

Hi Lars,

On 18/12/2019 17:09, Lars Kurth wrote:
> On 18/12/2019, 14:29, "Julien Grall" <julien@xxxxxxx> wrote:
>
>      Hi Lars,
>      
>      On 12/12/2019 21:14, Lars Kurth wrote:
>      > +### Workflow from an Author's Perspective
>      > +
>      > +When code authors receive feedback on their patches, they typically 
> first try
>      > +to clarify feedback they do not understand. For smaller patches or 
> patch series
>      > +it makes sense to wait until receiving feedback on the entire series 
> before
>      > +sending out a new version addressing the changes. For larger series, it 
> may
>      > +make sense to send out a new revision earlier.
>      > +
>      > +As a reviewer, you need some system that he;ps ensure that you address 
> all
>      
>      Just a small typo: I think you meant "helps" rather than "he;ps".
>      
>      Cheers,
>      
> Thank you: fixed in my working copy.
> One thing which occurred to me for reviews like these, where there is no ACK's 
> or Reviewed-by's is that I don't actually know whether you as reviewer is 
> otherwise happy with the remainder of patch.
> Normally the ACKed-by or Reviewed-by is a signal that it is
>  > I am assuming it is, but I think it may be worthwhile pointing this 
out in the document, that unless stated otherwise, the reviewer is happy 
with the patch
I don't think you can always assume this. There are case where I don't 
give a reviewed-by yet because I want to understand the follow-up 
patches first.
I think what Ian described correspond the best to my view here. And I 
agree that we probably want to be more explicit in the review to avoid 
confusion.
Cheers,

--
Julien Grall

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