Open Stack

For what is worth, I'm trying below to provide my perspective on Luke's
question both as a reviewer and as developer.

Salvatore


On 26 July 2014 20:02, Luke Gorrie <luke at snabb.co> wrote:

> On 25 July 2014 20:05, Stefano Maffulli <stefano at openstack.org> wrote:
>
>> Indeed, communication is key. I'm not sure how you envision to
>>  implement this though. We do send a message to first time
>> contributors[1] to explain them how the review process works and give
>> them very basic suggestions on how to react to comments (including what
>> to do if things seem stuck). The main issue here though is that few
>> people read emails, it's a basic fact of life.
>>
>
> That welcome message does seem to do a really good job of setting
> expectations.
>
> Can you explain more what you have in mind?
>>
>
> Here are some other topics that seem to take some time to develop a mental
> model of:
>
> How quickly and how often should you revise your patchset after a -1? (Is
> it better to give the community a week or so to collectively comment? Or
> should you revise ASAP after every negative review?)
>

This is a very hard question to answer. From a developer perspective often
you have tenths of outstanding patches, or the required changes translates
into a need of redoing your patch from scratch, so quickly addressing a -1
is not possible. On the other hand, from a reviewer perspective, if there
is too much time between two reviews then "memory" of previous reviews will
be lost and the review process will basically begin again from scratch.

Usually in order to help reviewers I try to be as pedant as possible in the
commit message. I also add comments on the patch to help reviewers
understand how I address their comments. This might help reviewers working
on your patch, and avoid questions which, despite being legitimate, will
just add more time to the review process for your patch.
I try to ping core reviewers on IRC as less as possible, unless there is
some sort of urgency from a community perspective. This is because I think
all core reviewers have their own review queue, and are typically already
working on sorting out reviews according to that queue.
However, if you can identify a core reviewer which has reviewed several
times your patch, then I guess it's ok to ping him directly, especially
when you are at the last stages of the review cycle and your patch just
needs a little push.


> How do you know if your change is likely to merge? (If you have had 15
> rounds of -1 votes and the last milestone deadline is a few days away,
> should you relax because your code is so thoroughly reviewed or should you
> despair because it should have been merged by now?)
>

If you have 15 -1 votes in just 1 round maybe you should consider
abandoning your patch!
Jokes apart, "-1" has a whole set of meanings. As a core reviewer I can -1
because:
- I think your approach is completely wrong and you should do your patch
again from scratch
- Your patch is ok but you should consider a more efficient implementation
- Somebody else has done a patch like yours
- The patch is ok but there are a few style/readability/maintainability
nits to address
- I have a possibly dumb question on the approach you've chosen and I would
like to discuss that with you before approving
Therefore the likeness of your patch merging depends on the specific nature
of the -1 you received.
I try to avoid -2s as much as possible. I put a -2 only when I reckon your
patch should never be merged because it'll make the software unstable or
tries to solve a problem that does not exist. -2s stick across patches and
tend to put off other reviewers.

Regarding your specific question, if after 15 rounds you still have a -1
that means you have at least a core reviewer constantly following it, so
you should probably be relaxed.


> In the final days before a merge deadline, would it be rude to "poke" the
> person responsible for merging, or would it be negligent not to?
>

Poking is never rude. However, poking a reviewer is not a guarantee that
your patch will be approved. The reviewer may add your patch to his/her
queue, but it might still slip through the cracks.
Most core reviewers two weeks before release time enter a "review crunch"
period where it's not unlikely for them to look at 10 to 20 patches per day.


> How do you decide which IRC meetings to attend? (For meetings that occur
> at difficult times outside of working hours in your timezone, when are you
> expected to attend them? Is it okay to focus on email/informal
> communication if that suits you better and gets the job done?)
>

Unfortunately there are too many meetings. I usually look at the agendas
for the meetings where I can be remotely interested in. If there's a
meeting where I think I have something to say, I will participate.
Otherwise I will read the meeting minutes and logs.


> If you're new to the project and you don't know anybody, who can you ask
> about this stuff?
>

I guess we'd just need to document that, but how this should be formalised
is probably more a question for the TC and the community manager.

>
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140728/1b6a27a8/attachment.html>