Skip to content

Commit e276184

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "Describe the use of Review-Priority label"
2 parents edaaa97 + b28579a commit e276184

File tree

1 file changed

+68
-0
lines changed

1 file changed

+68
-0
lines changed

doc/source/contributor/process.rst

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,74 @@ Getting that extra testing in place should stop a whole heap of bugs,
677677
again giving reviewers more time to get to the issues or features you
678678
want to add in the future.
679679

680+
What the Review-Priority label in Gerrit are use for?
681+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
682+
683+
A bit of history first. Nova used so called runway slots for multiple cycles.
684+
There was 3 slots, each can be filled with a patch series ready for review for
685+
two weeks at a time. We assumed that cores are focusing on reviewing the series
686+
while it is in the slot. We also assumed that the patch author is available and
687+
quickly fixing feedback while the series is in the slot. Meanwhile other
688+
patches waited in a FIFO queue for a free slot.
689+
690+
Our experience was:
691+
692+
1) It only worked if somebody kept the state of the queue and the slots up to
693+
date in the etherpad. So it needed a central authority to manage the
694+
process. This did not scale well.
695+
696+
2) It was as effective as we, cores, are kept it honest and allocated our
697+
review time on the patches in the slots. Such commitment is hard to get or
698+
follow up on without being aggressive.
699+
700+
So the aim of the new review priority process is to be as decentralized amongst
701+
cores as possible. We trust cores that when they mark something as priority
702+
then they also themselves commit to review the patch. We also assume that if a
703+
core reviewed a patch then that core should easily find another core as a
704+
second reviewer when needed.
705+
706+
Note that this process does not want to change how a patch is discovered to be
707+
ready for review. The patch authors free to you any existing forums and ways to
708+
get review attention.
709+
710+
Therefore we use the Review-Priority label in Gerrit in the following way:
711+
712+
* Review-Priority is a label with 0 or +1 values, that can be set by the
713+
members of the core team
714+
715+
* A core sets the Review-Priority flag to +1 to indicate that they will help
716+
the author to get the patch merged.
717+
718+
* We expect that the cores will limit the number of patches marked with +1
719+
Review-Priority based on their actual review bandwidth
720+
721+
* We expect that cores will check the list of reviews already having
722+
Review-Priority +1 set by other cores before they mark a new one as such to
723+
see where they can help first by being the second core.
724+
725+
* There will be a regular agenda point on the weekly meeting where the team
726+
look at the list of patches with +1 mark to keep an overall view what is
727+
happening in nova.
728+
729+
Pros:
730+
731+
* Decentralized
732+
733+
* Each core is responsible of its own commitments
734+
735+
* Review priority information is kept close to the review system
736+
737+
Cons:
738+
739+
* No externally enforced time limit on patches sitting idle with +1
740+
Review-Priority
741+
742+
* No externally enforced limit on how many things can be a priority at any
743+
given time.
744+
745+
* Does not (want to) solve the problem of discovering reviews that are ready to
746+
core review
747+
680748
Process Evolution Ideas
681749
=======================
682750

0 commit comments

Comments
 (0)