[teampractices] Code review social norms

[Rob Lanphier]

On Mon, Mar 14, 2016 at 12:40 PM, James Forrester <jforrester at wikimedia.org>
wrote:

> On 14 March 2016 at 19:15, Greg Grossmeier <greg at wikimedia.org> wrote:
>
>> Backgroun:
>> * This started as this task in our Phab:
>>   https://phabricator.wikimedia.org/T121751 "Document best practices to
>>   amend a change written by another contributor"
>> * Lot's of discussion there about what is "right" in the general sense.
>> * Mukunda found out a way of making everyone happy (maybe)
>> * Mukunda proposed that solution upstream, then Evan P wrote a long
>>   opinion piece on code review social contracts and basically concluded
>>   that our social contracts are toxic (a theme we keep hearing...)
>> ** here: https://secure.phabricator.com/T10584
>>
>> [​I  pretty strongly disagree with Evan] Indeed, I'd be more blunt: I
> think it's actively disrespectful to peers – be they WMF staff, third party
> staff, or volunteers alike – to *not* just tweak and merge their code.
> For instance, when there's a typo/misplaced whitespace in a comment or it
> needs a quick manual rebase, refusing to fix and merge isn't a great
> practice. People's time is precious and as reviewers we should all take on
> the burden of minor changes, and not give trivial C-1s (or Differential
> equivalents) that push the review cycle out for another hour/week/year
> (depending on the respondent's availability).
>

I fully agree with this.  It troubles me to openly and strongly disagree
with Evan, because I have so much respect for him as an upstream BDFL.  We
have a lot to learn from him when it comes to being a healthy upstream.

That said, there is a very unhealthy contrarian attitude toward the
competition (GitHub) which he is fostering.  If you look at the comment
thread on T10584, you'll see some snark about GitHub's model being
"broken".  Somehow, forcing developers to extra middleware like arc
*isn't* broken,
but GitHub is broken.  Riiiight.

In reading Evan's comments (and not other Phab users), his instincts seem
to come from the Facebook new developer mentoring model, and may not be
entirely wrong.  We need to evaluate whether we want to join him in this
battle against the prevailing attitude popularized by GitHub.  Personally,
I think it's a quixotic losing argument, but ultimately, WMF Release
Engineering needs to decide where they are going to place their loyalty and
support.

Rob
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/teampractices/attachments/20160315/60eb1e2b/attachment.html>