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

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



On 18.12.2019 18: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 there should ever be such an implication. Afaic there
are patches I comment upon, but that I either don't feel qualified
to give an ack/R-b on, or that I simply don't want to for whatever
reason. At best, no other comment (as in the above example) may be
taken as "I don't object".

Jan

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

 


Rackspace

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