[teampractices] Code review social norms

[Greg Grossmeier]

<quote name="Rob Lanphier" date="2016-03-15" time="18:37:18 -0700">
> > > 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.

I didn't mean to brush off either point. I actually think they are
legitimate points and we should work through them together, including
with Evan, because I think he has useful "not-us" perspective. And, as
he started his writeup upstream, is very open to being corrected and
having a conversation about it. I believe that having these kinds of
discussions with people outside of our community is healthy and
productive and will give us a better result in the end.

What I want to do is stop sniping each other and start collaboratively
finding solutions.

> 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?

I agree with it generally. And it's something that is possible and
has been in discussion on T121751, culminating (mostly) in
https://phabricator.wikimedia.org/T121751#2115258

> > 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.

It wasn't meant to, I was addressing the language choice, which I
believe (and still believe) was sub-par. I still think we should work
together on this. I can't simply come up with a new review system and
set of norms out of whole cloth. I would like to instead continue the
already fruitful conversations we've had to come to a useful and better
outcome (better than where we are now and better than any one person or
small group of people could do).

> 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.

That's a large statement that I disagree with the facts of.

I started this thread with a link to Evan's comments upstream in hopes
to start a discussion, which it did (critiques of Evan's points). I
generally prefer if people can interact directly, but I can and will, of
course, communicate needed information back upstream. I was not going to
do so very quickly as I wanted to wait until more people had a chance to
reply so that I could synthesize and then comment (instead of
copy/pasting each comment individually, for instance). This list tends
to have a higher response time than others, so I wanted to respect that.

> Are you asking one of us to take over your team's job as spokespeople to
> upstream?

Not at all.

> 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.

Which conversation? Mukunda sharing a fact on first patch? I don't see
why he couldn't share that information upstream. The task itself was
Evan proactively seeing us talk about it on our instance, seeing
Mukunda's work to make it a viable option for us (see above), and
starting a thread in his instance so that he can think through the
issues. I see that as a positive relationship and conversation.

> 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?

This presupposes that I don't, and I don't agree with that. I, and WMF
RelEng as a group, have done much charitable and constructive
representation with both upstream and within our community (see also:
the Dev Summit session which was an overwhelming success).

> > 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?

There was an explicit  act of support in the last ArchCom meeting I
attended where you offered to shepherd the migration RFC, which I took
as a show of support. I do still believe the work that WMF RelEng (and
others) have put into this migration deserves that support, yes.

Greg

-- 
| Greg Grossmeier            GPG: B2FA 27B1 F7EB D327 6B8E |
| identi.ca: @greg                A18D 1138 8E47 FAC8 1C7D |