Re: Experimenting with a shared review queue for Core::Build Config
On 11/10/2017 19:41, Botond Ballo wrote: On Wed, Oct 11, 2017 at 2:37 PM, Chris Cooperwrote: On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froyd 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. In case anyone hits this, you need to add "r?Build" for mozreview to detect the reviewer properly. Mark. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
On Fri, Oct 13, 2017 at 7:47 AM, Andreas Tolfsenwrote: > Also sprach smaug: > > How did the setup actually work? >> > > I think farre gave a satisfactory summary of the review tool above. > > I've asked this from farre too before, and IIRC, the reply was >> that is wasn't working that well. Certain devs still ended up >> doing majority of the reviews. But perhaps I misremember or >> certainly I don't know the details. >> > > Shared review queues isn’t a silver bullet balancing reviews > between peers for a couple of different reasons, but it arguably > makes it easier for peers to collaborate on (and be informed of) > reviews. > > For certain parts of the codebase, it is a sad fact the bus factor > is low and we shouldn’t be fooled that a system which practically > allows any one of one’s peers to pick up the review, will somehow > magically improve the turnaround time for patches in those areas. > You will still have reviews that can practically only be reviewed by > one single person who is the domain expert. > > On the flipside, when you have a patch for a piece of code multiple > peers know well and feel comfortable accepting, turnaround time > could be improved compared to the status quo where the single > r? could be travelling, on PTO, or otherwise preoccupied. This last point is what I think matters. As a build peer (for a subset of the build system), I know exactly who has the expertise to review my build system patches -- and I also know which of my patches don't require deep expertise and can be reviewed by any build peer. I manually balance my requests to keep this chaff out of the busy build peers' queues. I'm hoping the shared queue can let the set of build peers opt in to this balancing, keeping the simple patches out of the busy build peers' queues. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
Also sprach smaug: How did the setup actually work? I think farre gave a satisfactory summary of the review tool above. I've asked this from farre too before, and IIRC, the reply was that is wasn't working that well. Certain devs still ended up doing majority of the reviews. But perhaps I misremember or certainly I don't know the details. Shared review queues isn’t a silver bullet balancing reviews between peers for a couple of different reasons, but it arguably makes it easier for peers to collaborate on (and be informed of) reviews. For certain parts of the codebase, it is a sad fact the bus factor is low and we shouldn’t be fooled that a system which practically allows any one of one’s peers to pick up the review, will somehow magically improve the turnaround time for patches in those areas. You will still have reviews that can practically only be reviewed by one single person who is the domain expert. On the flipside, when you have a patch for a piece of code multiple peers know well and feel comfortable accepting, turnaround time could be improved compared to the status quo where the single r? could be travelling, on PTO, or otherwise preoccupied. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
On 10/11/2017 09:55 PM, Andreas Tolfsen wrote: +tools-marionette Also sprach Chris Cooper: Many of the build peers have long review queues. Is having a long review queue the actual issue? Isn't (too) high throughput at least equally bad issue. Does the new setup somehow try to ensure reviews actually do split more evenly among devs? (and that has nothing to do with review queue lengths) 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" core-build-config-revi...@mozilla.bugs 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. How did the setup actually work? I've asked this from farre too before, and IIRC, the reply was that is wasn't working that well. Certain devs still ended up doing majority of the reviews. But perhaps I misremember or certainly I don't know the details. I'm all for trying new setups to split review load more evenly and I'm very eager to hear how this experimentation works out! -Olli 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-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
+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" core-build-config-revi...@mozilla.bugs 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-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
On Wed, Oct 11, 2017 at 2:37 PM, Chris Cooperwrote: > On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froyd 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-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
On Wed, Oct 11, 2017 at 1:46 PM, Nathan Froydwrote: > 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-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Experimenting with a shared review queue for Core::Build Config
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 Cooperwrote: > 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" core-build-config-revi...@mozilla.bugs 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 > dev-bui...@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-builds ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Experimenting with a shared review queue for Core::Build Config
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" core-build-config-revi...@mozilla.bugs 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-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform