[xen 4.6 retrospective] Possible solution together with the comments will be helpful

[xen 4.6 retrospective] Possible solution together with the comments will be helpful

[Wu, Feng]

[-- Attachment #1.1: Type: text/plain, Size: 5494 bytes --]

= Issue / Observation =
Sometimes the review comments are quite open, it doesn't contain a possible solution or a clear direction,
so it is not clear for the contributor on how to effectively address them. At least, in Linux kernel and KVM side, if the maintainers have
objection to the implementation of the patches, they will give a possible solution or a direction which is very
helpful for the contributor to address the comments. Hence this will make the review discussion more effective and productive and save both reviewer and developer's time.

= Possible Solution / Improvement =
Try to give some possible solutions with the comments, especially for some big changes which affect a lot
to the whole patch-set.

Thanks,
Feng

From: xen-devel-bounces@lists.xen.org<mailto:xen-devel-bounces@lists.xen.org> [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of Lars Kurth
Sent: Tuesday, August 04, 2015 8:53 PM
To: xen devel
Subject: [Xen-devel] [xen 4.6 retrospective] Kicking off a retrospective for Xen 4.6 (deadline August 28th)


Hi all,

I have been asked by a number of people to kick off a retrospective for
* what worked well
* didn't work well
in the Xen 4.6 release cycle.

As most of the stress of the last few weeks is now out of the way, I
thought it would be appropriate to start this process now, and let it run
for a few weeks. We have not done this type of retrospective over e-mail
before (we have done so at developer meetings as well as Hackathons).
As we have a global and distributed community, I think it is worthwhile
trying this by email. To make this work, you need to follow a few ground
rules.

= Ground Rules =
We would like to hear
a) what worked well and
b) what didn't work well
in this release cycle.

Please provide feedback before August 28th.

== What to discuss / what not to discuss ==

* Do NOT send issues which are personal to the mailing list. This means,
do NOT send anything related to individuals or companies that may have
occurred in this release cycle. We do not want to have divisive flame
wars in our community: this does not fit our values and will help
no-one. If you believe that we do have an issue, that requires referring
to individuals/companies, please send me a private email following the
same formatting conventions as outlined below. I can then work with you
to figure out how to best approach this issue or piece of feedback.

* If you are not sure, contact me beforehand and we can discuss the best
way forward.

* It is entirely acceptable to discuss process issues in public. Examples
of topics that are suitable are: "the hard freeze did not work", "the hard
freeze did work well", "we should have longer freeze exception windows",
"we should have no freeze exceptions at all", "we should open master to
contributions after RC2", "we should rename freeze to something else as
it encourages misunderstandings", "we should have a shorter release cycle",
etc. - these are just examples.

* If you agree with an issue/solution you can reply to it with a comment
or a "+1" in the normal way

* If you disagree or have some concerns about what has been proposed,
feel free to reply. You can use the "-1 comment" format.

Please be MINDFUL when you reply to one of the ideas and suggestions that
were raised. The ground rule about NOT becoming personal also applies to
replies.

== Formatting of Mails ==

* Please only cover only *one* topic per feedback e-mail. In other
words please do not mix topics. This makes it easier to collate feedback
and for everyone to follow the threads.

* Use the [xen 4.6 retrospective] tag in the subject line to xen-devel@

* You can use the [good] or [bad] tag in the subject line to xen-devel@

* Make sure you CC community.manager@xenproject.org<mailto:community.manager@xenproject.org>

* Use [private] and the tags above, and do NOT send the mail to xen-devel@
if this is a question/issue ONLY directed at me. This will help me manage
my inbox

* Use [urgent], if your issue should be discussed at the upcoming
Developer Summit Aug 18-19, OR if it is something which affects the current
release cycle (e.g. "we should open master to contributions after RC2")

* Use a descriptive subject line describing the issue/feedback, e.g.
"release cadence was too long", ...

* If you can, reply to this e-mail thread and change the title as
described above. That way, most of the feedback will be nicely threaded

== Content ==
Ideally, you would follow the following template

---
  Subject: [xen 4.6 retrospective] ...

  = Issue / Observation =
  Describe what you have noticed
  Described whether this was positive or negative and how
  ...

  = Possible Solution / Improvement =
  Describe how you think we could improve the situation
  Explain why your proposal would make a positive impact
  ...
---

== Closing the Discussion ==

I will monitor the post-mortem and I will act as facilitator: for example
I may call out improper behaviour in public or private to keep the
retrospective focussed.

At some point after August 28th, we will look at the issues and
suggestions raised and see which ones we can address and how. For the
ones we can address, we will condense feedback into concrete proposals
which we may put forward for voting (unless everyone agrees that something
is a great idea, or strictly speaking no vote is necessary).

Best Regards
Lars

[-- Attachment #1.2: Type: text/html, Size: 23800 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [xen 4.6 retrospective] Possible solution together with the comments will be helpful

[Jan Beulich]

>>> On 31.08.15 at 10:24, <feng.wu@intel.com> wrote:
> = Issue / Observation =
> Sometimes the review comments are quite open, it doesn't contain a possible 
> solution or a clear direction,
> so it is not clear for the contributor on how to effectively address them. 
> At least, in Linux kernel and KVM side, if the maintainers have
> objection to the implementation of the patches, they will give a possible 
> solution or a direction which is very
> helpful for the contributor to address the comments. Hence this will make 
> the review discussion more effective and productive and save both reviewer 
> and developer's time.
> 
> = Possible Solution / Improvement =
> Try to give some possible solutions with the comments, especially for some 
> big changes which affect a lot
> to the whole patch-set.

I think when a solution can be thought of in the context of reviewing,
it is being given. I believe I know which case you allude to here, and
I'm afraid it's not always reasonable for the reviewer(s) to do the
contributor's work of finding a solution when none is obvious.

Jan

Re: [xen 4.6 retrospective] Possible solution together with the comments will be helpful

[Lars Kurth]

> On 31 Aug 2015, at 09:33, Jan Beulich <jbeulich@suse.com> wrote:
> 
>>>> On 31.08.15 at 10:24, <feng.wu@intel.com> wrote:
>> = Issue / Observation =
>> Sometimes the review comments are quite open, it doesn't contain a possible 
>> solution or a clear direction,
>> so it is not clear for the contributor on how to effectively address them. 
>> At least, in Linux kernel and KVM side, if the maintainers have
>> objection to the implementation of the patches, they will give a possible 
>> solution or a direction which is very
>> helpful for the contributor to address the comments. Hence this will make 
>> the review discussion more effective and productive and save both reviewer 
>> and developer's time.

I am wondering how big the difference between linux/kvm and xen is. And if there is a big difference (taking Jan's comment below into account), what the root cause is.

>> 
>> = Possible Solution / Improvement =
>> Try to give some possible solutions with the comments, especially for some 
>> big changes which affect a lot
>> to the whole patch-set.
> 
> I think when a solution can be thought of in the context of reviewing,
> it is being given. I believe I know which case you allude to here, and
> I'm afraid it's not always reasonable for the reviewer(s) to do the
> contributor's work of finding a solution when none is obvious.

Again, maybe in this case, it would be helpful for the reviewer to call out the fact that there is no obvious solution for a problem he/she has raised. In that case, it gives the co-maintener (of which we have more now at least in core areas) an opportunity to step up. 

There is of course also a learning angle: to some degree, educating and helping new contributors, should eventually make them better, which in the long term leads to more efficient reviews. So to some degree, the upfront investment by a reviewer in educating a new contributor, will lead to less work for the reviewer in the long run. 

Re: [xen 4.6 retrospective] Possible solution together with the comments will be helpful

[Wei Liu]

On Mon, Aug 31, 2015 at 02:33:22AM -0600, Jan Beulich wrote:
> >>> On 31.08.15 at 10:24, <feng.wu@intel.com> wrote:
> > = Issue / Observation =
> > Sometimes the review comments are quite open, it doesn't contain a possible 
> > solution or a clear direction,
> > so it is not clear for the contributor on how to effectively address them. 
> > At least, in Linux kernel and KVM side, if the maintainers have
> > objection to the implementation of the patches, they will give a possible 
> > solution or a direction which is very
> > helpful for the contributor to address the comments. Hence this will make 
> > the review discussion more effective and productive and save both reviewer 
> > and developer's time.
> > 
> > = Possible Solution / Improvement =
> > Try to give some possible solutions with the comments, especially for some 
> > big changes which affect a lot
> > to the whole patch-set.
> 
> I think when a solution can be thought of in the context of reviewing,
> it is being given. I believe I know which case you allude to here, and
> I'm afraid it's not always reasonable for the reviewer(s) to do the
> contributor's work of finding a solution when none is obvious.
> 

I don't follow everything close enough, so I don't comment on specific
case.

While I agree it's contributors' job to present a viable solution, fresh
contributors might not have the whole picture of the project, they still
need clearer direction on how to proceed. I.e. what general direction
should be taken, what precise constraints they need to meet.  Sometimes,
IMHO, saying something is not going to work, is not good enough for
contributors to proceed, especially when the changes are non-trivial.

This applies even more when a viable solution is not obvious, and to
work out a viable solutions involves changing some core infrastructure
and breaking some previous held assumptions.

Wei.

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

Re: [xen 4.6 retrospective] Possible solution together with the comments will be helpful

[Andrew Cooper]

On 31/08/15 09:33, Jan Beulich wrote:
>>>> On 31.08.15 at 10:24, <feng.wu@intel.com> wrote:
>> = Issue / Observation =
>> Sometimes the review comments are quite open, it doesn't contain a possible 
>> solution or a clear direction,
>> so it is not clear for the contributor on how to effectively address them. 
>> At least, in Linux kernel and KVM side, if the maintainers have
>> objection to the implementation of the patches, they will give a possible 
>> solution or a direction which is very
>> helpful for the contributor to address the comments. Hence this will make 
>> the review discussion more effective and productive and save both reviewer 
>> and developer's time.
>>
>> = Possible Solution / Improvement =
>> Try to give some possible solutions with the comments, especially for some 
>> big changes which affect a lot
>> to the whole patch-set.
> I think when a solution can be thought of in the context of reviewing,
> it is being given. I believe I know which case you allude to here, and
> I'm afraid it's not always reasonable for the reviewer(s) to do the
> contributor's work of finding a solution when none is obvious.

Personally, I always try to state clearly when I can't suggest a solution.

Having said that, it is easy to make assumptions about the way a
reviewee will interpret a comment.  I expect this most likely comes down
to existing knowledge of related areas, meaning that the same review
comment might be fine for one contributor, and confusing to another.

Again, I do my best to try and be as clear as I can, but I am aware that
I don't always manage it.  My apologies if this is the case.

If anything is not clear, the best action is to ask a direct question on
the relevant thread, even if it as simple as "I am sorry, but I do not
understand this comment".  I certainly, and I expect all reviewers, will
be far happier answering a question to aid with clarity, than for the
contributor to go away and have a stab in the dark and come back with a
v$N+1 which most likely needs further work.

~Andrew

Re: [xen 4.6 retrospective] Possible solution together with the comments will be helpful

[George Dunlap]

On Mon, Aug 31, 2015 at 9:33 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 31.08.15 at 10:24, <feng.wu@intel.com> wrote:
>> = Issue / Observation =
>> Sometimes the review comments are quite open, it doesn't contain a possible
>> solution or a clear direction,
>> so it is not clear for the contributor on how to effectively address them.
>> At least, in Linux kernel and KVM side, if the maintainers have
>> objection to the implementation of the patches, they will give a possible
>> solution or a direction which is very
>> helpful for the contributor to address the comments. Hence this will make
>> the review discussion more effective and productive and save both reviewer
>> and developer's time.
>>
>> = Possible Solution / Improvement =
>> Try to give some possible solutions with the comments, especially for some
>> big changes which affect a lot
>> to the whole patch-set.
>
> I think when a solution can be thought of in the context of reviewing,
> it is being given. I believe I know which case you allude to here, and
> I'm afraid it's not always reasonable for the reviewer(s) to do the
> contributor's work of finding a solution when none is obvious.

It's not necessarily the *reviewer's* job to suggest an alternate
implementation; but it is the *maintainer's* job, as architect and
caretaker of a subsystem, to try to figure out how new features can be
incorporated into the code.

If there's not an obvious immediate solution, then the maintainer
should at very least say something like, "I'm not happy with this way
of doing things.  I can't immediately think of an alternative -- let
me give it some thought and come back to it."

I've been on the receiving side of a relationship where modus operandi
was "I'm not happy with this, but I'm not going to tell you what I
want instead, you figure it out", and I can tell you that the "How
about this?" game is not at all a fun game to play.

 -George