Experimenting with a shared review queue for Core::Build Config

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Experimenting with a shared review queue for Core::Build Config

Chris Cooper-4
Many of the build peers have long review queues. I'm not convinced
that all of the review requests going to any particular build peer
need to be exclusive. We're going to try an experiment to see if we
can make this better for patch authors and reviewers alike. To this
end, we've set up a shared review queue for patches submitted to the
Core::Build Config module.

How to participate:

When you submit your next Build Config patch, simply select the new,
shared "user" [hidden email] as the reviewer
instead of a specific build peer. The build peers are watching this
new user, and will triage and review your patch accordingly.

This new arrangement should hopefully shorten patch queues for
specific reviewers and improve turnaround times for everybody. It also
has the added benefit of automatically working around absences,
vacations, and departures of build peers.

Note: this system still allows for targeting reviews to specific build
peers. Indeed, the build peers may do exactly that in triage if the
patch in question touches a particular sub-domain. However, I would
encourage patch authors to use the shared bucket unless you really
understand the sub-domain yourself or are collaborating directly with
a particular build peer.

As indicated in the subject, this is an experiment. I will monitor
patch queues and turnaround time over the next few months, and then
decide in January whether we should continue or try something else.

Thanks for your patience, and for trying something new.

cheers,
--
coop
_______________________________________________
dev-builds mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-builds
Reply | Threaded
Open this post in threaded view
|

Re: Experimenting with a shared review queue for Core::Build Config

Nathan Froyd-5
Does this user have a bugzilla :alias so that folks submitting patches
via MozReview or similar can just write r=build-peer or something,
rather than having to manually select the appropriate shared queue
after submitting their patch for review?

-Nathan

On Wed, Oct 11, 2017 at 1:41 PM, Chris Cooper <[hidden email]> wrote:

> Many of the build peers have long review queues. I'm not convinced
> that all of the review requests going to any particular build peer
> need to be exclusive. We're going to try an experiment to see if we
> can make this better for patch authors and reviewers alike. To this
> end, we've set up a shared review queue for patches submitted to the
> Core::Build Config module.
>
> How to participate:
>
> When you submit your next Build Config patch, simply select the new,
> shared "user" [hidden email] as the reviewer
> instead of a specific build peer. The build peers are watching this
> new user, and will triage and review your patch accordingly.
>
> This new arrangement should hopefully shorten patch queues for
> specific reviewers and improve turnaround times for everybody. It also
> has the added benefit of automatically working around absences,
> vacations, and departures of build peers.
>
> Note: this system still allows for targeting reviews to specific build
> peers. Indeed, the build peers may do exactly that in triage if the
> patch in question touches a particular sub-domain. However, I would
> encourage patch authors to use the shared bucket unless you really
> understand the sub-domain yourself or are collaborating directly with
> a particular build peer.
>
> As indicated in the subject, this is an experiment. I will monitor
> patch queues and turnaround time over the next few months, and then
> decide in January whether we should continue or try something else.
>
> Thanks for your patience, and for trying something new.
>
> cheers,
> --
> coop
> _______________________________________________
> dev-builds mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-builds
_______________________________________________
dev-builds mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-builds
Reply | Threaded
Open this post in threaded view
|

Re: Experimenting with a shared review queue for Core::Build Config

Chris Cooper-4
On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froyd <[hidden email]> wrote:
> Does this user have a bugzilla :alias so that folks submitting patches
> via MozReview or similar can just write r=build-peer or something,
> rather than having to manually select the appropriate shared queue
> after submitting their patch for review?

I see this as an added efficiency, so I'll see how much effort it is
to set this up. The people behind MozReview are explicitly not
expending effort there right now in favor of phabricator though.

cheers,
--
coop
_______________________________________________
dev-builds mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-builds
Reply | Threaded
Open this post in threaded view
|

Re: Experimenting with a shared review queue for Core::Build Config

Botond Ballo
On Wed, Oct 11, 2017 at 2:37 PM, Chris Cooper <[hidden email]> wrote:
> On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froyd <[hidden email]> wrote:
>> Does this user have a bugzilla :alias so that folks submitting patches
>> via MozReview or similar can just write r=build-peer or something,
>> rather than having to manually select the appropriate shared queue
>> after submitting their patch for review?
>
> I see this as an added efficiency, so I'll see how much effort it is
> to set this up. The people behind MozReview are explicitly not
> expending effort there right now in favor of phabricator though.

It does not require any setup on the MozReview side - you just need to
add ":build-peer" (or whatever the chosen alias) to the "Real name"
field of the user's Bugzilla profile, and MozReview will autodetect
it.

Cheers,
Botond
_______________________________________________
dev-builds mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-builds
Reply | Threaded
Open this post in threaded view
|

Re: Experimenting with a shared review queue for Core::Build Config

Andreas Tolfsen-2
In reply to this post by Chris Cooper-4
+tools-marionette

Also sprach Chris Cooper:

> Many of the build peers have long review queues. I'm not convinced
> that all of the review requests going to any particular build peer
> need to be exclusive.

I think it is great that you’re experimenting with this, and I’m
very excited about it!

In addition to peers doing review triage to balance out review
queues, there’s an argument to be made that for a certain category
of work we do, who reviews the code is of secondary importance.

Speaking from my own personal experience, the vast majority of
patches I write could feasibly be reviewed by any one of the peers
of Testing :: Marionette.  There is certainly the odd occasion
when I need to explicitly draw from the expertise of a particular
individual, but I have found this is more the exception than the
norm in the line of work I do on the remote protocol.

In a small team it can also be tricky to keep track of who is
around on any given day across multiple continents and timezones.
A first-come-first-served review system I think would also help
smaller teams with not-so-big review queues improve turnaround times
for patches.

Improving the time from patch written, through review, to
integration in mozilla-central, is doubly important when we receive
patches from new contributors who don’t know who to flag.

> When you submit your next Build Config patch, simply select the
> new, shared "user" [hidden email] as the
> reviewer instead of a specific build peer. The build peers are
> watching this new user, and will triage and review your patch
> accordingly.

I would love to see the modules I’m peer of participate in the
same experiment.  Can I ask you to elaborate what a ‘user’ is in
this context and how practically this ‘triage’ happens?

> This new arrangement should hopefully shorten patch queues for
> specific reviewers and improve turnaround times for everybody. It
> also has the added benefit of automatically working around
> absences, vacations, and departures of build peers.

When I worked for Opera we had a similar system where a pool of
reviewers and watchers would get notified about incoming reviews.
In the review tool you would save a glob-filter stating that you
were interested in either reviewing or “watching”, meaning you
only cared about the notification, a change in a certain subsection
of the repository.

More importantly, new contributors to the codebase didn’t have
to know who to flag for review.  They’d submit the patch and the
review tool would figure out who to notify.  The first respondent
reviewer would then, similarly to how you describe, triage the
change to the most appropriate person, or if it was a simple change,
simply do the review him- or herself.

In the danger of derailing this thread to talk about the new review
tool that is meant to replace Splinter and MozReview, I would be
absolutely thrilled if it had support for “pooled review queues”
like this.  If there was a way to annotate directories with OWNER
files or similar it would be even better.

From what I understand—and please feel free to correct me—the
“shared user” you talk about is simply another Bugzilla account?
That is great for experimentation, but it it turns out a success,
having better ergonomics would be desirable.
_______________________________________________
dev-builds mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-builds