[teampractices] Patch review culture of Wikimedia teams

[Kevin Smith]

Perhaps we should split this into "code review tied to a phab task" vs.
"code review that does not have an associated phab task".

TPG probably has no visibility into the latter (if those even exist).

The former would be handled normally by my (and probably other TPGer's)
regular processes. The task would either get stuck in the "In Progress" or
"Under review" column, and after a day or two, we would start asking the
team why it isn't moving. In the case of Discovery, Dan would often notice
it and start asking about it even before I would.

So I think the answer to the first part of your original question would be:
Yes, we would encourage regular review grooming (via phab tasks). The
process for Discovery describes the use of the "Needs Review" column, but
doesn't go into specifics about WIP limits or how long something can sit
there before being considered a problem. As I said, that's about 2 days for
us.

I'm not aware of any data about teams that do better or worse about this.
Discovery wouldn't objectively distinguish between internal and external
contributions, but could make subjective calls about whether to push harder
on something either because it's a critical internal thing, OR because we
would want to avoid disappointing a volunteer.



Kevin Smith
Agile Coach, Wikimedia Foundation


On Fri, Mar 18, 2016 at 5:21 AM, Andre Klapper <aklapper at wikimedia.org>
wrote:

> Hej,
>
> thanks for the reply!
>
> On Tue, 2016-03-15 at 17:20 -0700, Kevin Smith wrote:
> > TPG generally focuses on process and inter-personal issues, and
> > doesn't get into the tech itself.
> > So our focus tends to be at the phab task level, rather than at the
> > gerrit patch level.
>
> As engineering tasks in Phabricator often require a code change going
> through review, I'd consider code review a part of the process to get
> Phabricator tasks resolved.
> Technical implementation details ("Gerrit") shouldn't be the main point
> of my request - it's rather the social aspect of interaction between
> authors and reviewers, and potentially documenting best practices
> across teams performing code review as part of their usual processes.
>
> Do you agree with that understanding?
>
> > I think most TPGers happen to be embedded into teams that receive
> > relatively few external code submissions.
>
> Let's ignore the "external contributions" aspect as I'd expect many
> aspects of code review to also apply when reviewing internal
> contributions.
>
> Thanks,
> andre
>
> > I wonder whether integrating code review into phab will give us some
> > new tools to help teams more easily monitor the flow of patches. I
> > don't know what tools are available in gerrit.
> >
> >
> >
> > On Mon, Mar 14, 2016 at 6:19 AM, Andre Klapper <aklapper at wikimedia.org>
> wrote:
> > > Thanks everybody for the comments! However I'm still curious if this is
> > > part of the TPG scope, hence I'd welcome a reply from TPG members.
> > >
> > > Thanks,
> > > andre
> > >
> > > On Mon, 2016-03-07 at 14:16 +0100, Andre Klapper wrote:
> > > > https://phabricator.wikimedia.org/T101686 lists "Prioritization
> / weak
> > > > open source culture: more pressure to write new code than to review
> > > > patches contributed."
> > > >
> > > > Apart from whether that statement is true or not:
> > > > Does the Team Practices Group encourage regular Gerrit patch backlog
> > > > grooming? If so, how, and is there any documentation available, or
> even
> > > > data which teams perform better or worse? Is there any
> differentiation
> > > > between "internal" patches by team members vs. contributed patches?
> > > > Or is this out of scope for TPG?
>
> --
> Andre Klapper | Wikimedia Bugwrangler
> http://blogs.gnome.org/aklapper/
>
>
>
> _______________________________________________
> teampractices mailing list
> teampractices at lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/teampractices
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.wikimedia.org/pipermail/teampractices/attachments/20160318/30dda8ce/attachment.html>