Repository navigation
[PR Bot] Select single main and backup reviewer using label priority - #40315
Conversation
8076128 to
ce3b166
Compare
|
I had an LLM do some quick analysis on how this PR would impact review distribution, here was the outcome: Simulation: Proposed Reviewer Assignment (
|
| Label | Priority | Pool Size | Current: Times Tagged | Current: Expected Reviews | Proposed: Times Tagged | Proposed: Expected Reviews |
|
|
|---|---|---|---|---|---|---|---|---|
spanner |
1 | 1 | 10.00 | 3.46 | 6.67 | 6.67 | -3.33 | +3.21 |
bigtable |
1 | 2 | 14.00 | 4.47 | 9.67 | 9.67 | -4.33 | +5.20 |
kafka |
1 | 3 | 19.00 | 6.49 | 14.00 | 14.00 | -5.00 | +7.51 |
delta |
1 | 2 | 13.00 | 4.17 | 10.33 | 10.33 | -2.67 | +6.16 |
iceberg |
1 | 2 | 31.00 | 13.53 | 27.33 | 27.33 | -3.67 | +13.80 |
yaml |
1 | 2 | 37.00 | 12.67 | 28.00 | 28.00 | -9.00 | +15.33 |
Go |
2 | 3 | 201.00 | 190.96 | 192.00 | 192.00 | -9.00 | +1.04 |
Python |
2 | 5 | 279.00 | 233.74 | 227.50 | 227.50 | -51.50 | -6.24 |
Java |
2 | 4 | 274.00 | 210.17 | 186.50 | 186.50 | -87.50 | -23.67 |
Build |
3 | 4 | 170.00 | 146.22 | 132.50 | 132.50 | -37.50 | -13.72 |
website |
3 | 4 | 54.00 | 38.12 | 29.50 | 29.50 | -24.50 | -8.62 |
no-matching-label (fallback) |
— | 7 | 136.00 | 136.00 | 136.00 | 136.00 | 0.00 | 0.00 |
| Total | 1,238.00 | 1,000.00 | 1,000.00 | 1,000.00 | -238.00 (-19.2%) | 0.00 |
Multi-Label & Tie Breakdown Across the 1,000 PRs
- Current Config (# of reviewers tagged per PR):
1reviewer: 856 PRs2reviewers: 90 PRs3reviewers: 32 PRs4reviewers: 13 PRs5+reviewers: 9 PRs (6 PRs with 5, 1 PR with 6, 2 PRs with 9)
- Proposed Config (# of labels tied at highest priority per PR):
- Deterministic (
1label): 961 PRs - Random choice among
2tied labels: 28 PRs - Random choice among
3tied labels: 9 PRs - Random choice among
4tied labels: 2 PRs
- Deterministic (
2. Summary Statistics by Person
Sorted by proposed assignments (descending):
| Reviewer | Labels in Config | Current: Times Tagged | Current: Expected Reviews | Proposed: Times Tagged | Proposed: Expected Reviews |
|
|
|---|---|---|---|---|---|---|---|
shunping |
Go, Python, website, fallback
|
155.10 | 139.00 | 135.94 | 135.94 | -19.17 | -3.06 |
jrmccluskey |
Go, Python, fallback
|
139.35 | 126.65 | 125.69 | 125.69 | -13.67 | -0.96 |
damccorm |
Python, Build, website, fallback
|
135.44 | 115.54 | 108.44 | 108.44 | -27.00 | -7.10 |
chamikaramj |
delta, iceberg, yaml, Java, fallback
|
138.15 | 93.58 | 107.93 | 107.93 | -30.22 | +14.35 |
Abacn |
Java, Build, website, fallback
|
143.57 | 117.84 | 105.95 | 105.95 | -37.62 | -11.89 |
kennknowles |
Java, website, fallback
|
104.90 | 83.31 | 74.31 | 74.31 | -30.60 | -9.00 |
tvalentyn |
Python, fallback
|
75.77 | 66.66 | 65.23 | 65.23 | -10.54 | -1.43 |
lostluck |
Go |
67.50 | 64.07 | 64.42 | 64.42 | -3.08 | +0.35 |
ahmedabu98 |
delta, iceberg, Java
|
81.25 | 54.49 | 58.94 | 58.94 | -22.31 | +4.45 |
claudevdm |
Python |
56.95 | 48.30 | 47.24 | 47.24 | -9.71 | -1.06 |
derrickaw |
yaml, Build
|
55.00 | 40.55 | 43.67 | 43.67 | -11.33 | +3.11 |
Amar3tto |
Build |
42.00 | 35.61 | 31.92 | 31.92 | -10.08 | -3.69 |
nielm |
spanner |
10.00 | 3.46 | 6.67 | 6.67 | -3.33 | +3.21 |
igorbernstein2 |
bigtable |
7.50 | 2.48 | 5.33 | 5.33 | -2.17 | +2.85 |
Dippatel98 |
kafka |
6.83 | 2.39 | 5.17 | 5.17 | -1.67 | +2.78 |
johnjcasey |
kafka |
6.83 | 2.39 | 5.17 | 5.17 | -1.67 | +2.78 |
mutianf |
bigtable |
6.50 | 1.98 | 4.33 | 4.33 | -2.17 | +2.35 |
sjvanrossum |
kafka |
5.33 | 1.72 | 3.67 | 3.67 | -1.67 | +1.95 |
| Total | 1,238.00 | 1,000.00 | 1,000.00 | 1,000.00 | -238.00 | 0.00 |
3. Key Takeaways
- Eliminates 238 redundant reviewer tags (-19.2%): Every single reviewer in
.github/REVIEWERS.ymlreceives fewer@tags under the proposed configuration because PRs with multiple labels no longer ping 2–9 people simultaneously. - Routes I/O & component PRs to domain reviewers instead of generalists:
- Previously, almost every
Priority 1PR (kafka,yaml,iceberg,spanner,bigtable,delta) also carried aJavaorPythonlabel (and oftenBuild), so generalists onJava/Python/Buildwere co-tagged alongside the component owner. - Under
#40315,Javatags drop by 87.5 (-31.9%),Pythontags drop by 51.5 (-18.5%),Buildtags drop by 37.5 (-22.1%), andwebsitetags drop by 24.5 (-45.4%), while expected reviews performed shift from broad SDK/infra pools toward the specific component owners (+51.2expected reviews across the six Priority 1 labels).
- Previously, almost every
kennknowles
left a comment
There was a problem hiding this comment.
Seems like a safe step with good ROI to me
|
TBH I think it is a small enough step you can take it to dev@ or not, but it is also easy to back out of if you merge it at the same time and it is unpopular. I suspect no one will particularly notice if you just do it, as we will be getting PR assignees roughly the same as before, except there will just be one, and they will be the more specific person. |
I was planning on getting lazy consensus (basically, speak now if you object or see issues) |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
Stacked on #40313.
Instead of assigning reviewers for every matching label on a PR, chooses a single main reviewer from the highest-priority matching label configured in
.github/REVIEWERS.ymland includes an untagged backup reviewer from the same label (falling back tofallbackReviewersif the label only has one eligible reviewer) so the PR author knows how to escalate.