Re: [Xen-devel] [v10][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-07-22 at 08:33 +0800, Chen, Tiejun wrote:
> On 2015/7/21 23:57, Ian Jackson wrote:
> 
> > It's true that I didn't comment on the frat that you had half-done one
> > of the things I had requested.  It is of course a waste of my time to
> > be constantly re-reviewing half-done changes.

> Next time I should see each line of all comments carefully. Maybe its
> good way to use IRC to take your quick advice in advance,

The solution to this is to be systematic in how you handle the email
based review of a series, not to add a further to the reviewer by using
IRC as well.

For example, here is how I handle review of a series I am working on:

I keep all the replies to a series I'm working on marked unread. When I
come to rework the series I take the first reply to the first patch and
start a draft reply to it quoting the entire review.

I then go through the comments one by one and either:

      * make the _complete_ code change, including adding the "Changes
        in vN" bit to the commit log and delete that comment from the
        reply
or
      * write a reply in my draft to that particular comment which does
        one or more of:

              * Explain that I don't understand the suggestion,
                probably asking questions to try and clarify what was
                being asked for.
              * Explain, with reasons, why I disagree with the
                suggestion
              * Explain, with reasons, why I only implemented part of
                the suggestion.

Only then do I move on to the next comment in that mail and repeat. At
the end I've either deleted all the comments from my draft (because
I've fully implemented everything) so the draft can be discarded or I
have an email to send explaining what I've not done and why. Only now
do I mark the original review email as read.

Then I move on to the next reply to that thread in my mail box and
repeat until I have been through every mail in the thread and addressed
_all_ of the comments.

At the end of this process _every_ bit of review feedback is addressed
either by making the requested change or by responding explaining the
reason why the change hasn't been made. This is absolutely crucial. You
should never silently ignore a piece of review, even if you don't
understand it or disagree with it, always reply and explain why you
haven't.

If the review was particularly long or complex I will then do a second
pass through the review comments and check that every comment is either
mentioned in a "Changes in vN" changelog comment or I have replied to
it.

I do all of that before posting the next version. IMHO until a
contributor has shown they are diligent about addressing review
comments they should _never_ send out a series which only has review
partially addressed.

The presence of an IRC channel in no way changes the requirement to be
systematic and thorough when dealing with email review.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel