Skip to content

Block pull requests as long as changes are requested #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 12 additions & 4 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ export const getMilestones = async (): Promise<Milestone[]> => {
return Object.values(earliestPatchVersions);
};

export const getPrApprovers = async (prNumber: number) => {
export const getPrReviewers = async (
prNumber: number,
): Promise<{ approvers: Set<string>; blockers: Set<string> }> => {
// load all reviews
const reviews: {
state:
Expand All @@ -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<string>();
const blockers = new Set<string>();
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 (
Expand Down Expand Up @@ -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`,
{
Expand Down
6 changes: 3 additions & 3 deletions src/github_test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { assertEquals } from "https://deno.land/[email protected]/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"]),
Expand All @@ -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);
}),
);
});
Expand Down
28 changes: 19 additions & 9 deletions src/lgtm.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
addLabels,
getPrApprovers,
getPrReviewers,
removeLabel,
setCommitStatus,
} from "./github.ts";
Expand All @@ -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
Expand Down Expand Up @@ -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<string>; blockers: Set<string> },
) => {
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";
}

Expand Down