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

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



Thanks for your clarification to me.

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

Are you saying this case of resending this particular patch online?

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.

Yes, in this case we're arguing, I was really trying to send a sample of this code fragment to ask this before I sent out the complete change.

               * 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

I should double check each line but sometimes this doesn't mean I can understand everything correctly to do right thing as you expect. But this is really not what I intend to do.

Thanks
Tiejun

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


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


 


Rackspace

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