diff --git a/README.md b/README.md index ba7d871..933f9c5 100644 --- a/README.md +++ b/README.md @@ -42,9 +42,11 @@ included in any milestone. ### LGTM The script will maintain each pull request's LGTM count. It will add the -appropriate label (one of `lgtm/need 2`, `lgtm/need 1`, or `lgtm/done`) based on -the number of approvals the pull request has. It will also set the commit status -to `success` if the pull request has 2 or more approvals (`pending` if not). +appropriate label (one of `lgtm/need 2`, `lgtm/need 1`, `lgtm/done`, or +`lgtm/blocked`) based on the number of approvals (or change requests) the pull +request has. It will also set the commit status to `success` if the pull request +has 2 or more approvals without changes requested (`pending` if not or `failure` +if changes are requested). ## Usage diff --git a/src/github.ts b/src/github.ts index 2091149..8e8bbb7 100644 --- a/src/github.ts +++ b/src/github.ts @@ -220,7 +220,9 @@ export const getMilestones = async (): Promise => { return Object.values(earliestPatchVersions); }; -export const getPrApprovers = async (prNumber: number) => { +export const getPrReviewers = async ( + prNumber: number, +): Promise<{ approvers: Set; blockers: Set }> => { // load all reviews const reviews: { state: @@ -244,23 +246,29 @@ export const getPrApprovers = async (prNumber: number) => { page++; } - // count approvers by replaying all reviews (they are already sorted) + // count approvers and blockers by replaying all reviews (they are already sorted) const approvers = new Set(); + const blockers = new Set(); for (const review of reviews) { switch (review.state) { case "APPROVED": approvers.add(review.user.login); + blockers.delete(review.user.login); break; case "DISMISSED": + approvers.delete(review.user.login); + blockers.delete(review.user.login); + break; case "CHANGES_REQUESTED": approvers.delete(review.user.login); + blockers.add(review.user.login); break; default: break; } } - return approvers; + return { approvers, blockers }; }; export const createBackportPr = async ( @@ -331,7 +339,7 @@ export const createBackportPr = async ( ); // request review from original PR approvers - const approvers = await getPrApprovers(originalPr.number); + const { approvers } = await getPrReviewers(originalPr.number); await fetch( `${GITHUB_API}/repos/go-gitea/gitea/pulls/${json.number}/requested_reviewers`, { diff --git a/src/github_test.ts b/src/github_test.ts index 8f5f77c..067ba59 100644 --- a/src/github_test.ts +++ b/src/github_test.ts @@ -1,7 +1,7 @@ import { assertEquals } from "https://deno.land/std@0.185.0/testing/asserts.ts"; -import { fetchBranch, getPrApprovers } from "./github.ts"; +import { fetchBranch, getPrReviewers } from "./github.ts"; -Deno.test("getPrApprovers() returns the appropriate approvers", async () => { +Deno.test("getPrReviewers() returns the appropriate approvers", async () => { const prToApprovers = { 23993: new Set(["delvh", "jolheiser"]), 24051: new Set(["delvh", "silverwind"]), @@ -13,7 +13,7 @@ Deno.test("getPrApprovers() returns the appropriate approvers", async () => { }; await Promise.all( Object.entries(prToApprovers).map(async ([pr, approvers]) => { - assertEquals(await getPrApprovers(Number(pr)), approvers); + assertEquals((await getPrReviewers(Number(pr))).approvers, approvers); }), ); }); diff --git a/src/lgtm.ts b/src/lgtm.ts index f8ecc69..2a1804d 100644 --- a/src/lgtm.ts +++ b/src/lgtm.ts @@ -1,6 +1,6 @@ import { addLabels, - getPrApprovers, + getPrReviewers, removeLabel, setCommitStatus, } from "./github.ts"; @@ -14,15 +14,15 @@ export const setPrStatusAndLabel = async ( number: number; }, ) => { - let approvers; + let reviewers; try { - approvers = await getPrApprovers(pr.number); + reviewers = await getPrReviewers(pr.number); } catch (error) { console.error(error); return; } - const { state, message, desiredLabel } = getPrStatusAndLabel(approvers.size); + const { state, message, desiredLabel } = getPrStatusAndLabel(reviewers); const currentLgtmLabels = pr.labels.filter((l) => l.name.startsWith("lgtm/")); // remove any undesired lgtm labels @@ -64,18 +64,28 @@ export const setPrStatusAndLabel = async ( }; // returns the status, message, and label for a given number of approvals -export const getPrStatusAndLabel = (approvals: number) => { +export const getPrStatusAndLabel = ( + reviewers: { approvers: Set; blockers: Set }, +) => { let desiredLabel = "lgtm/need 2"; let message = "Needs two more approvals"; - let state: "pending" | "success" = "pending"; + let state: "pending" | "success" | "failure" = "pending"; + + if (reviewers.blockers.size > 0) { + desiredLabel = "lgtm/blocked"; + message = "Blocked by " + Array.from(reviewers.blockers).join(", "); + state = "failure"; + return { state, message, desiredLabel }; + } - if (approvals === 1) { + if (reviewers.approvers.size === 1) { desiredLabel = "lgtm/need 1"; message = "Needs one more approval"; } - if (approvals >= 2) { + + if (reviewers.approvers.size >= 2) { desiredLabel = "lgtm/done"; - message = `Approved by ${approvals} people`; + message = `Approved by ${reviewers.approvers.size} people`; state = "success"; }