Skip to content

Commit ea5bfb9

Browse files
ryanclarkryanclark
authored
Dismiss other reviews on approval, add design/group 2 reviewers (#61)
1 parent 06fcd82 commit ea5bfb9

File tree

5 files changed

+227
-40
lines changed

5 files changed

+227
-40
lines changed

‎.changeset/many-planets-begin.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@gravitational/design-system': patch
3+
---
4+
5+
Another test for testing the GitHub release

.github/workflows/bot.yml renamed to .github/workflows/changeset.yml

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,6 @@ env:
1313
NODE_VERSION: 22.x
1414

1515
jobs:
16-
assign-reviewer:
17-
name: Assign random reviewer
18-
runs-on: ubuntu-latest
19-
if: ${{ !github.event.pull_request.draft }}
20-
21-
steps:
22-
- name: Checkout
23-
uses: actions/checkout@v5
24-
25-
- name: Setup pnpm
26-
uses: ./.github/actions/setup-pnpm
27-
with:
28-
node-version: ${{ env.NODE_VERSION }}
29-
30-
- name: Assign random reviewer
31-
env:
32-
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
33-
run: |
34-
pnpm bot reviewer \
35-
${{ github.repository_owner }} \
36-
${{ github.event.repository.name }} \
37-
${{ github.event.pull_request.number }}
38-
3916
changeset-bot:
4017
name: Check for changeset
4118
runs-on: ubuntu-latest

‎.github/workflows/reviewers.yml‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
name: Bot
2+
3+
on:
4+
pull_request_review:
5+
type: [ submitted, edited, dismissed ]
6+
pull_request:
7+
types: [ opened, ready_for_review, synchronize, reopened ]
8+
9+
permissions:
10+
contents: read
11+
pull-requests: write
12+
issues: write
13+
14+
env:
15+
NODE_VERSION: 22.x
16+
17+
jobs:
18+
check-reviewers:
19+
name: Check reviewers
20+
runs-on: ubuntu-latest
21+
if: ${{ !github.event.pull_request.draft }}
22+
23+
steps:
24+
- name: Checkout
25+
uses: actions/checkout@v5
26+
27+
- name: Setup pnpm
28+
uses: ./.github/actions/setup-pnpm
29+
with:
30+
node-version: ${{ env.NODE_VERSION }}
31+
32+
- name: Check reviewers
33+
env:
34+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
35+
run: |
36+
pnpm bot reviewer \
37+
${{ github.repository_owner }} \
38+
${{ github.event.repository.name }} \
39+
${{ github.event.pull_request.number }}

‎bot/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ async function main() {
3939
await runReleaseCommand(octokit, {
4040
owner,
4141
repo,
42-
version: args[2],
43-
tar_gz_path: args[3],
42+
version: prNumber,
43+
tar_gz_path: action,
4444
});
4545

4646
return;

‎bot/review.ts‎

Lines changed: 181 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,15 @@ import { Octokit } from '@octokit/rest';
44
import { resolveErrorMessage } from './util.ts';
55

66
const GROUP1_REVIEWERS = ['ryanclark'];
7-
const GROUP2_REVIEWERS = ['strideynet'];
7+
const GROUP2_REVIEWERS = [
8+
'bl-nero',
9+
'ravicious',
10+
'rudream',
11+
'mcbattirola',
12+
'nicholasmarais1158',
13+
'michellescripts',
14+
];
15+
const DESIGN_REVIEWERS = ['roraback'];
816

917
export async function runReviewerCommand(
1018
octokit: Octokit,
@@ -25,19 +33,9 @@ export async function runReviewerCommand(
2533
return;
2634
}
2735

28-
const existingReviewers = await octokit.rest.pulls.listRequestedReviewers({
29-
owner: params.owner,
30-
repo: params.repo,
31-
pull_number: params.pull_number,
32-
});
36+
const labels = pullRequest.data.labels.map(l => l.name);
3337

34-
if (
35-
existingReviewers.data.users.length > 0 ||
36-
existingReviewers.data.teams.length > 0
37-
) {
38-
core.info('PR already has reviewers assigned, skipping random assignment');
39-
return;
40-
}
38+
const needsDesignReview = labels.includes('needs-design-review');
4139

4240
const reviews = await octokit.rest.pulls.listReviews({
4341
owner: params.owner,
@@ -46,26 +44,194 @@ export async function runReviewerCommand(
4644
});
4745

4846
const prAuthor = pullRequest.data.user.login;
47+
4948
const humanReviews = reviews.data.filter(
5049
review =>
5150
review.user &&
5251
!review.user.login.includes('[bot]') &&
5352
review.user.login !== prAuthor
5453
);
5554

56-
if (humanReviews.length > 0) {
55+
const existingReviewers = await octokit.rest.pulls.listRequestedReviewers({
56+
owner: params.owner,
57+
repo: params.repo,
58+
pull_number: params.pull_number,
59+
});
60+
61+
if (needsDesignReview) {
62+
const alreadyRequested = new Set(
63+
existingReviewers.data.users.map(u => u.login)
64+
);
65+
66+
const alreadyReviewed = new Set(
67+
humanReviews.map(r => r.user?.login).filter(Boolean) as string[]
68+
);
69+
70+
const designToRequest = DESIGN_REVIEWERS.filter(
71+
r => r !== prAuthor && !alreadyRequested.has(r) && !alreadyReviewed.has(r)
72+
);
73+
74+
if (designToRequest.length > 0) {
75+
try {
76+
await octokit.rest.pulls.requestReviewers({
77+
owner: params.owner,
78+
repo: params.repo,
79+
pull_number: params.pull_number,
80+
reviewers: designToRequest,
81+
});
82+
83+
core.info(
84+
`Requested design review from: ${designToRequest.join(', ')}`
85+
);
86+
} catch (error) {
87+
core.error(
88+
`Error requesting design reviewers: ${resolveErrorMessage(error)}`
89+
);
90+
}
91+
} else {
92+
core.info(
93+
'No eligible design reviewers to request (or already requested).'
94+
);
95+
}
96+
}
97+
98+
const hasAnyHumanReview = humanReviews.length > 0;
99+
const hasAnyRequestedReviewer =
100+
existingReviewers.data.users.length > 0 ||
101+
existingReviewers.data.teams.length > 0;
102+
103+
if (!hasAnyHumanReview && !hasAnyRequestedReviewer) {
104+
await assignRandomReviewers(octokit, {
105+
owner: params.owner,
106+
repo: params.repo,
107+
pull_number: params.pull_number,
108+
pr_author: prAuthor,
109+
});
110+
} else if (hasAnyHumanReview) {
57111
core.info('PR already has reviews, skipping random assignment');
112+
} else if (hasAnyRequestedReviewer) {
113+
core.info('PR already has reviewers assigned, skipping random assignment');
114+
}
115+
116+
const approvedBy = new Set(
117+
humanReviews
118+
.filter(r => r.state === 'APPROVED')
119+
.map(r => r.user?.login)
120+
.filter(Boolean) as string[]
121+
);
122+
123+
const group1Approved = GROUP1_REVIEWERS.some(u => approvedBy.has(u));
124+
const group2Approved = GROUP2_REVIEWERS.some(u => approvedBy.has(u));
125+
const designApproved = DESIGN_REVIEWERS.some(u => approvedBy.has(u));
126+
127+
const eligibleGroup1 = GROUP1_REVIEWERS.filter(u => u !== prAuthor);
128+
const eligibleGroup2 = GROUP2_REVIEWERS.filter(u => u !== prAuthor);
129+
const eligibleDesign = DESIGN_REVIEWERS.filter(u => u !== prAuthor);
130+
131+
const hasEligibleGroup1 = eligibleGroup1.length > 0;
132+
133+
const failureReasons: string[] = [];
134+
135+
if (hasEligibleGroup1) {
136+
if (!group1Approved || !group2Approved) {
137+
failureReasons.push(
138+
`Required approvals missing: ${!group1Approved ? 'group 1' : ''}${!group1Approved && !group2Approved ? ' and ' : ''}${!group2Approved ? 'group 2' : ''}.`
139+
);
140+
}
141+
} else {
142+
if (!group2Approved) {
143+
failureReasons.push('Needs group 2 approval.');
144+
}
145+
}
146+
147+
if (needsDesignReview && !designApproved) {
148+
failureReasons.push('Design approval required.');
149+
}
150+
151+
if (failureReasons.length > 0) {
152+
const fmt = (arr: string[]) => (arr.length ? arr.join(', ') : 'none');
153+
154+
core.error('Eligible approvers');
155+
156+
if (!group1Approved && eligibleGroup1.length > 0) {
157+
core.error(`Group 1: ${fmt(eligibleGroup1)}`);
158+
}
159+
160+
if (!group2Approved) {
161+
core.error(`Group 2: ${fmt(eligibleGroup2)}`);
162+
}
163+
164+
if (needsDesignReview) {
165+
core.error(`Design: ${fmt(eligibleDesign)}`);
166+
}
167+
168+
core.setFailed(`Approval policy not met: ${failureReasons.join(' ')}`);
169+
58170
return;
59171
}
60172

61-
await assignRandomReviewers(octokit, {
173+
await dismissOtherReviewsIfApproved(octokit, {
62174
owner: params.owner,
63175
repo: params.repo,
64176
pull_number: params.pull_number,
65177
pr_author: prAuthor,
178+
reviews: humanReviews,
66179
});
67180
}
68181

182+
async function dismissOtherReviewsIfApproved(
183+
octokit: Octokit,
184+
params: {
185+
owner: string;
186+
repo: string;
187+
pull_number: number;
188+
pr_author: string;
189+
reviews: {
190+
id: number;
191+
state: string;
192+
user?: { login: string } | null;
193+
}[];
194+
}
195+
) {
196+
const toDismiss = params.reviews.filter(
197+
r =>
198+
r.state !== 'APPROVED' &&
199+
r.state !== 'DISMISSED' &&
200+
r.user &&
201+
!r.user.login.includes('[bot]') &&
202+
r.user.login !== params.pr_author
203+
);
204+
205+
if (toDismiss.length === 0) {
206+
core.info('There are approvals but no other human reviews to dismiss');
207+
return;
208+
}
209+
210+
core.info(
211+
`Found approval; dismissing ${toDismiss.length} other review(s): ` +
212+
toDismiss.map(r => r.user?.login).join(', ')
213+
);
214+
215+
for (const review of toDismiss) {
216+
try {
217+
await octokit.rest.pulls.dismissReview({
218+
owner: params.owner,
219+
repo: params.repo,
220+
pull_number: params.pull_number,
221+
review_id: review.id,
222+
message: 'Dismissing non-approved review because the PR is approved.',
223+
});
224+
core.info(`Dismissed review ${review.id} (${review.user?.login})`);
225+
} catch (error) {
226+
core.error(
227+
`Failed to dismiss review ${review.id} (${review.user?.login}): ${resolveErrorMessage(
228+
error
229+
)}`
230+
);
231+
}
232+
}
233+
}
234+
69235
async function assignRandomReviewers(
70236
octokit: Octokit,
71237
params: {

0 commit comments

Comments
 (0)