[llvm-commits] [llvm] r134907 - /llvm/trunk/utils/TableGen/

[Evan Cheng]

This doesn't work. People are very busy, reviewing patches is but a small part of everyone's daily work.

If you keep the patches small and incremental, then it's perfectly ok to commit and have the patches reviewed post commit. But that's not what's happening here.

Evan

On Jul 12, 2011, at 12:14 PM, David A. Greene wrote:

> greened at obbligato.org (David A. Greene) writes:
> 
>> I'm all for code review, especially on complex patches.  That's why I
>> sent this one for review ahead of time.  But the submitter needs to get
>> at least some indication someone is looking at it in a timely manner.
>> Otherwise the submitter is completely in the dark.
> 
> I think what might work is the following:
> 
> - Within 16 working hours after a review request is sent, the relevant
>  code owner either sends a review or sends an acknowledgement that the
>  request was received.  Either type of response should indicate that
>  the responder is the code owner (so the sumitter knows who has final
>  say).
> 
>  [I realize that in this case I did not wait 24 working hours.  That
>   was a screw-up on my part and I apologize.]
> 
> - If the reviewer needs more time to do the review, he or she will
>  provide an estimated time to complete the review.  Extensions to that
>  timeframe should be communicated as soon as possible once they are
>  known.  The idea here is to keep the submitter in the loop.
> 
> - If, after 24 hours, the submitter has not received any
>  acknowledgement, he or she sends a single ping to call attention to
>  the review request.
> 
> - Following the ping, the submitter should wait an additional eight
>  working hours.  If there is no response forthcoming, the submitter may
>  commit the patch which is then subject to post-commit review.
> 
> - A ping should almost never be necessary.
> 
> This will at least give a predictable timeframe for the submitter.  As
> it is now, people who submit code are lost as to when things might move
> forward.  That makes scheduling and planning very difficult.
> 
> Does this sound reasonable?
> 
>                                 -Dave
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits