[teampractices] Apologies to Greg (Re: Code review social norms)

Wed Mar 16 22:48:02 UTC 2016

Hi folks,

Thanks to everyone who privately pointed out that my email to Greg had an
overly sarcastic and belittling tone.  Greg, sorry.  :-(  To everyone else,
my apologies for how my email negatively affected the tone of this list;
please call me out on it if I do something like this again.

Greg and I are still working out what the status of T119908
<https://phabricator.wikimedia.org/T119908#1903835>, and what my offer to
shepherd it signals.  I'll strive to be better about AGF when trying to
understand a disagreement, and be more patient about the course of private
conversations before letting them spill onto public mailing lists.

Rob

On Tue, Mar 15, 2016 at 6:37 PM, Rob Lanphier <robla at wikimedia.org> wrote:

> On Tue, Mar 15, 2016 at 11:26 AM, Greg Grossmeier <greg at wikimedia.org>
> wrote:
>
>> <quote name="Rob Lanphier" date="2016-03-15" time="11:02:39 -0700">
>> > 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.
>>
>> Snark both ways doesn't help.
>>
>
> Nor does brushing off my point (and James' point) by sanctimoniously
> dismissing my point as snark.
>
> You failed to bring any clarity to this conversation with this
> interjection.  I'm starting to come around to Yuri's point that he made in
> the T119908 conversation starting the end of December
> <https://phabricator.wikimedia.org/T119908#1903835>.
>
> I'll requote one of James' more powerful statements: "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)."  Can you please respond to this?
>
>
> > 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.
>>
>> I don't think it's a matter of "placing one's loyalty" as you term it.
>> This isn't a war nor an election. It's a highly complex and nuanced issue
>> of social norms and practices that is highly dependent upon
>> circumstances and location. So no, I reject this characterization.
>
>
> Declaring this "complex and nuanced" doesn't bring clarity to it, and
> doesn't address James' points.  WMF Release Engineering is expressing its
> strong preference for moving from Gerrit to Differential.  You seem to be
> abdicating your team's responsibility as diplomats between this user
> community and the Phab upstream, and basically begging us to help you make
> an argument you are having a difficult time expressing yourself.
>
> Are you asking one of us to take over your team's job as spokespeople to
> upstream?  I'll note that the upstream conversation on Upstream:T10584
> <https://secure.phabricator.com/T10584> includes the following comment
> from Mukunda:
>
> > It took 6 months before I got my first patch accepted in gerrit. This is
> not an exageration.
> > At my previous job I deployed to production within the first week.
>
> Greg, do you believe that upstream's tracker is the best place for us to
> have that particular conversation?  I don't mind having this conversation
> publicly, but it seems more appropriate for us to discuss this in a
> Wikimedia venue.
>
> Gergo and Subbu made excellent points on T10584, and I attempted to make
> my point more constructively.  Can your team represent our development
> community and development processes a little more charitably and
> constructively?
>
> Instead, WMF Release Engineering will continue to work with all parties
>> to help think through code-review best practices, along with TPG and
>> the Architecture Committee as leaders/supporters.
>
>
> It seems a little presumptuous to expect ArchCom's support.  Do you
> believe that you deserve it?
>
> Rob
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/teampractices/attachments/20160316/c04e86be/attachment-0001.html>