codehaus


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

[nova][ptg] Review culture (was: Ussuri scope containment)


Thanks for the responses, all.

This subthread is becoming tangential to my original purpose, so I'm
renaming it.

>> The best way to get reviews is to lurk in IRC and beg.
<snip>
> When I joined I was taught that instead of begging go and review open
> patches which a) helps the review load of dev team b) makes you known
> in the community. Both helps getting reviews on your patches. Does it
> always work? No. Do I like begging for review? No. Do I like to get
> repatedly pinged to review? No. So I would suggest not to declare that
> the only way to get review is to go and beg.

I recognize I was generalizing; begging isn't really "the best way" to
get reviews. Doing reviews and becoming known (and *then* begging :) is
far more effective -- but is literally impossible for many contributors.
Even if they have the time (percentage of work week) to dedicate
upstream, it takes massive effort and time (calendar) to get there. We
can not and should not expect this of every contributor.

More...

On 10/1/19 8:00 AM, Jeremy Stanley wrote:
> On 2019-10-01 08:38:50 -0400 (-0400), Tom Barron wrote:
> [...]
>> In projects I have worked on there is no need to encourage extra
>> begging and squeaky wheel prioritization has IMO not been a
>> healthy thing.
>>
>> There is no better way to get ones reviews stalled than to beg for
>> reviews with patches that are not close to ready for review and at
>> the same time contribute no useful reviews oneself.
>>
>> There is nothing wrong with pinging to get attention to a review
>> if it is ready and languishing, or if it solves an urgent issue,
>> but even in these cases a ping from someone who doesn't "cry wolf"
>> and who has built a reputation as a contributor carries more
>> weight.
> [...]
> 
> Agreed, it drives back to Eric's comment about familiarity with the
> team's reviewer culture. Just saying "hey I pushed these patches can
> someone look" is often far less effective for a newcomer than "I
> reported a bug in subsystem X which is really aggravating and review
> 654321 fixes it if anyone has a moment to look" or "tbarron: I
> addressed your comments on review 654321 when you get a chance to
> revisit your -1, thanks!"
> 
> My cardinal rules of begging: Don't mention the nicks of random
> people who have not been involved in the change unless you happen to
> actually know it's one they'll personally be interested in. Provide
> as much context as possible (within reason) to attract the actual
> interest of potential reviewers. Be polite, thank people, and don't
> assume your change is important to anyone nor that there's someone
> who has time to look at it. And most important, as you noted too, if
> you're waiting around then take a few minutes and go review
> something to pass the time! ;)
> 

This is *precisely* the kind of culture that we cannot expect
inexperienced contributors to understand. We can write it down [1], but
then we have to get people to read what's written.

To tie back to the original thread, this is where it would help to have
a core (or experienced dev) as a mentor/liaison to be the first point of
contact for questions, guidance, etc. Putting it in the spec process
ensures it doesn't get missed (like a doc sitting "out there" somewhere).

efried

[1] though I fear that would end up being a long-winded and wandering
tome, difficult to read and grok, assuming we could even agree on what
it should say (frankly, there are some aspects we should be embarrassed
to admit in writing)