From 9e4cbda4772a9f92623a3b71c8a810d286fd8cc1 Mon Sep 17 00:00:00 2001
From: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Date: Wed, 19 Jun 2024 09:54:36 -0600
Subject: [PATCH] ci: Automatically add milestones to prs and issues (#44357)
* automatically add milestones to prs and issues
* test code
* test code
* test commit (#3138)
* test commit (#43320)
* test commit (#44357)
* remove test code
* add newlines
* prefer older version milestones
---
.github/workflows/check-milestone.yml | 77 +++-------
release/src/github.ts | 2 +-
release/src/index.ts | 1 +
release/src/linked-issues.ts | 10 ++
release/src/linked-issues.unit.spec.ts | 26 +++-
release/src/milestones.ts | 185 +++++++++++++++++++++++++
release/src/milestones.unit.spec.ts | 74 ++++++++++
release/src/types.ts | 20 ++-
release/tsconfig.json | 3 +-
9 files changed, 331 insertions(+), 67 deletions(-)
create mode 100644 release/src/milestones.ts
create mode 100644 release/src/milestones.unit.spec.ts
diff --git a/.github/workflows/check-milestone.yml b/.github/workflows/check-milestone.yml
index 032819ec835..bd9e8ab7cbd 100644
--- a/.github/workflows/check-milestone.yml
+++ b/.github/workflows/check-milestone.yml
@@ -1,86 +1,41 @@
-name: Milestone Reminder
+name: Automatically set milestone
+run-name: Set milestone for ${{ github.event.head_commit.message }}
on:
- pull_request:
- types: [closed]
+ push:
branches:
- - "master"
+ - release-x.**
jobs:
- milestone-reminder:
- if: github.event.pull_request.merged == true
- name: Remind to set milestone
+ set-milestone:
+ name: Automatically set milestone
runs-on: ubuntu-22.04
timeout-minutes: 5
permissions:
pull-requests: write
- issues: read
+ issues: write
+ contents: read
steps:
- uses: actions/checkout@v4
with:
sparse-checkout: release
+ ref: master # we want the logic from master, even though we're triggering on a release branch
- name: Prepare build scripts
run: cd ${{ github.workspace }}/release && yarn && yarn build
- uses: actions/github-script@v7
with:
script: | # js
- const { getLinkedIssues } = require('${{ github.workspace }}/release/dist/index.cjs');
+ const { setMilestoneForCommits } = require('${{ github.workspace }}/release/dist/index.cjs');
const owner = context.repo.owner;
const repo = context.repo.repo;
- const pullNumber = '${{ github.event.pull_request.number }}';
+ const branchName = '${{ github.ref }}';
+ const commitMessage = '${{ github.event.head_commit.message }}';
- function getMilestone(pullOrIssue) {
- // pull has pull.data.milestone, issue has issue.milestone
- return pullOrIssue?.data?.milestone || pullOrIssue?.milestone;
- }
-
- const pull = await github.rest.pulls.get({
+ await setMilestoneForCommits({
+ github,
owner,
repo,
- pull_number: pullNumber,
+ branchName,
+ commitMessages: [commitMessage],
});
-
- if (getMilestone(pull)) {
- console.log("Pull request has milestone", pull.data.milestone?.title);
- process.exit(0);
- }
-
- // the github API doesn't expose linked issues, so we have to parse the body ourselves
- const prDescription = pull.data.body;
- const linkedIssues = getLinkedIssues(prDescription);
-
- if (Array.isArray(linkedIssues) && linkedIssues.length > 0) {
- console.log('linked issue(s) found');
-
- linkedIssues.forEach(async (issueId, index) => {
- const issue = await github.rest.issues.get({
- owner,
- repo,
- issue_number: issueId,
- });
-
- const milestone = getMilestone(issue);
-
- if (milestone) {
- console.log(`Linked issue #${issueId} has milestone`, milestone.title);
- index === linkedIssues.length -1 && process.exit(0);
- } else {
- console.log("No milestone found");
-
- const author = pull.data.user.login;
- const guideLink = "https://www.notion.so/metabase/Metabase-Branching-Strategy-6eb577d5f61142aa960a626d6bbdfeb3?pvs=4#3dea255ffa3b4f74942a227844e889fa";
- const message = `@${author} Did you forget to add a milestone to the issue #${issueId} linked in this PR? _[When and where should I add a milestone?](${guideLink})_`;
-
- await github.rest.issues.createComment({
- owner,
- repo,
- issue_number: pullNumber,
- body: message,
- });
-
- // Exit as soon as we found an issue without a milestone and alerted the author.
- process.exit(0);
- }
- });
- }
diff --git a/release/src/github.ts b/release/src/github.ts
index 6a131d1c2d8..7b98e0eaf26 100644
--- a/release/src/github.ts
+++ b/release/src/github.ts
@@ -7,7 +7,7 @@ import {
isValidVersionString,
} from "./version-helpers";
-const getMilestones = async ({
+export const getMilestones = async ({
github,
owner,
repo,
diff --git a/release/src/index.ts b/release/src/index.ts
index 42266358774..7e38d880a90 100644
--- a/release/src/index.ts
+++ b/release/src/index.ts
@@ -1,6 +1,7 @@
export * from "./backports";
export * from "./github";
export * from "./linked-issues";
+export * from "./milestones";
export * from "./release-notes";
export * from "./release-status";
export * from "./slack";
diff --git a/release/src/linked-issues.ts b/release/src/linked-issues.ts
index 7e5fe232796..c76892720eb 100644
--- a/release/src/linked-issues.ts
+++ b/release/src/linked-issues.ts
@@ -12,3 +12,13 @@ export function getLinkedIssues(body: string) {
return null;
}
+
+export function getPRsFromCommitMessage(message: string) {
+ const result = [ ...message.matchAll(/\(#(\d+)\)/g) ];
+ if (!result.length) {
+ console.log('no pr found in commit message', message);
+ return null;
+ }
+
+ return result.map(r => Number(r[1]));
+}
diff --git a/release/src/linked-issues.unit.spec.ts b/release/src/linked-issues.unit.spec.ts
index d589dd7524f..b832b4522d1 100644
--- a/release/src/linked-issues.unit.spec.ts
+++ b/release/src/linked-issues.unit.spec.ts
@@ -1,4 +1,4 @@
-import { getLinkedIssues } from "./linked-issues";
+import { getLinkedIssues, getPRsFromCommitMessage } from "./linked-issues";
const closingKeywords = [
"Close",
@@ -107,3 +107,27 @@ describe("getLinkedIssues", () => {
});
});
});
+
+describe("getPRsFromCommitMessage", () => {
+ it("should return `null` when no PR is found", () => {
+ expect(getPRsFromCommitMessage("")).toBeNull();
+ expect(getPRsFromCommitMessage("Lorem ipsum dolor sit amet.")).toBeNull();
+ expect(getPRsFromCommitMessage("Fix #123.")).toBeNull();
+ expect(getPRsFromCommitMessage("Fix#123.")).toBeNull();
+ expect(getPRsFromCommitMessage("Fix # 123.")).toBeNull();
+ expect(getPRsFromCommitMessage("123 456")).toBeNull();
+ expect(getPRsFromCommitMessage("123 #456)")).toBeNull();
+ expect(getPRsFromCommitMessage("123 (#456")).toBeNull();
+ expect(getPRsFromCommitMessage("123 (#456.99)")).toBeNull();
+ });
+
+ it("should return the PR id for a single pr backport", () => {
+ expect(getPRsFromCommitMessage("Backport (#123)")).toEqual([123]);
+ expect(getPRsFromCommitMessage("Backport (#123456) !")).toEqual([123456]);
+ });
+
+ it("should return the PR id for a message with multiple backport PRs", () => {
+ expect(getPRsFromCommitMessage("Backport (#123) (#456)")).toEqual([123, 456]);
+ expect(getPRsFromCommitMessage("Backport (#1234) and (#4567)")).toEqual([1234, 4567]);
+ });
+});
diff --git a/release/src/milestones.ts b/release/src/milestones.ts
new file mode 100644
index 00000000000..ee0804ab67e
--- /dev/null
+++ b/release/src/milestones.ts
@@ -0,0 +1,185 @@
+import { getMilestones } from "./github";
+import { getLinkedIssues, getPRsFromCommitMessage } from "./linked-issues";
+import type { Issue, GithubProps, Milestone } from "./types";
+import {
+ getMajorVersion,
+ getVersionFromReleaseBranch,
+} from "./version-helpers";
+
+function isBackport(pullRequest: Issue) {
+ return pullRequest.title.includes('backport') ||
+ (
+ Array.isArray(pullRequest.labels) &&
+ pullRequest.labels.some((label) => label.name === 'was-backported')
+ );
+}
+
+// for auto-setting milestones, we don't ever want to auto-set a patch milestone
+// which we release VERY rarely
+function ignorePatches(version: string) {
+ return version.split('.').length < 4;
+}
+
+function versionSort(a: string, b: string) {
+ const [aMajor, aMinor] = a.split('.').map(Number);
+ const [bMajor, bMinor] = b.split('.').map(Number);
+
+ if (aMajor !== bMajor) {
+ return aMajor - bMajor;
+ }
+
+ if (aMinor !== bMinor) {
+ return aMinor - bMinor;
+ }
+
+ return 0;
+}
+
+const isNotNull = <T>(value: T | null): value is T => value !== null;
+
+
+async function getOriginalPR({
+ github,
+ repo,
+ owner,
+ pullRequestNumber,
+}: GithubProps & { pullRequestNumber: number }) {
+ // every PR in the release branch should have a pr number
+ // it could be a backport PR or an original PR
+ const pull = await github.rest.pulls.get({
+ owner,
+ repo,
+ pull_number: pullRequestNumber,
+ });
+
+ if (pull?.data && isBackport(pull.data)) {
+ return getOriginalPR({
+ github,
+ repo,
+ owner,
+ pullRequestNumber: pull.data.number
+ });
+ }
+
+ const linkedIssues = await getLinkedIssues(pull.data.body ?? '');
+
+ if (linkedIssues) {
+ console.log('found linked issue for PR', pull.data.number, linkedIssues);
+ return linkedIssues.map(Number);
+ }
+ console.log("no linked issues found in PR body", pull.data.number);
+ return [pull.data.number];
+}
+
+async function setMilestone({ github, owner, repo, issueNumber, milestone }: GithubProps & { issueNumber: number, milestone: Milestone }) {
+ // we can use this for both issues and PRs since they're the same for many purposes in github
+ const issue = await github.rest.issues.get({
+ owner,
+ repo,
+ issue_number: issueNumber,
+ });
+
+ if (!issue.data.milestone) {
+ return github.rest.issues.update({
+ owner,
+ repo,
+ issue_number: issueNumber,
+ milestone: milestone.number,
+ });
+ }
+
+ const existingMilestone = issue.data.milestone;
+
+ if (existingMilestone.number === milestone.number) {
+ console.log(`Issue ${issueNumber} is already tagged with this ${milestone.title} milestone`);
+ return;
+ }
+
+ const existingMilestoneIsNewer = versionSort(existingMilestone.title, milestone.title) > 0;
+
+ // if existing milestone is newer, change it
+ if (existingMilestoneIsNewer) {
+ console.log(`Changing milestone from ${existingMilestone.title} to ${milestone.title}`);
+
+ await github.rest.issues.update({
+ owner,
+ repo,
+ issue_number: issueNumber,
+ milestone: milestone.number,
+ });
+ }
+
+ const commentBody = existingMilestoneIsNewer
+ ? `🚀 This should also be released by [v${existingMilestone.title}](${existingMilestone.html_url})`
+ : `🚀 This should also be released by [v${milestone.title}](${milestone.html_url})`;
+
+ console.log(`Adding comment to issue ${issueNumber} that already has milestone ${existingMilestone.title}`);
+
+ return github.rest.issues.createComment({
+ owner,
+ repo,
+ issue_number: issueNumber,
+ body: commentBody,
+ });
+}
+
+// get the next open milestone (e.g. 0.57.8) for the given major version (e.g 57)
+export function getNextMilestone(
+ { openMilestones, majorVersion }:
+ { openMilestones: Milestone[], majorVersion: number | string }
+): Milestone | undefined {
+ const milestonesForThisMajorVersion = openMilestones
+ .filter(milestone => milestone.title.startsWith(`0.${majorVersion}`))
+ .filter(milestone => ignorePatches(milestone.title))
+ .sort((a, b) => versionSort(a.title, b.title));
+
+ const nextMilestone = milestonesForThisMajorVersion[0];
+
+ return nextMilestone;
+}
+
+export async function setMilestoneForCommits({
+ github,
+ owner,
+ repo,
+ branchName,
+ commitMessages,
+}: GithubProps & { commitMessages: string[], branchName: string}) {
+ // figure out milestone
+ const branchVersion = getVersionFromReleaseBranch(branchName);
+ const majorVersion = getMajorVersion(branchVersion);
+ const openMilestones = await getMilestones({ github, owner, repo });
+ const nextMilestone = getNextMilestone({ openMilestones, majorVersion });
+
+ if (!nextMilestone) {
+ throw new Error(`No open milestone found for major version ${majorVersion}`);
+ }
+
+ console.log('Next milestone:', nextMilestone.title);
+
+ // figure out issue or PR
+ const PRsToCheck = commitMessages.flatMap(getPRsFromCommitMessage).filter(isNotNull);
+
+ if (!PRsToCheck.length) {
+ throw new Error('No PRs found in commit messages');
+ }
+
+ console.log(`Checking ${PRsToCheck.length} PRs for issues to tag`);
+
+ const issuesToTag = [];
+
+ for (const prNumber of PRsToCheck) { // for loop to avoid rate limiting
+ issuesToTag.push(...(await getOriginalPR({
+ github,
+ owner,
+ repo,
+ pullRequestNumber: prNumber,
+ })));
+ }
+
+ console.log(`Tagging ${issuesToTag.length} issues with milestone ${nextMilestone.title}`)
+
+ for (const issueNumber of issuesToTag) { // for loop to avoid rate limiting
+ await setMilestone({ github, owner, repo, issueNumber, milestone: nextMilestone });
+ }
+}
diff --git a/release/src/milestones.unit.spec.ts b/release/src/milestones.unit.spec.ts
new file mode 100644
index 00000000000..059a07844b7
--- /dev/null
+++ b/release/src/milestones.unit.spec.ts
@@ -0,0 +1,74 @@
+import { getNextMilestone } from "./milestones";
+import type { Milestone } from "./types";
+
+describe('milestones', () => {
+ const testMilestones = [
+ { number: 577, title: '0.57.7' },
+ { number: 578, title: '0.57.8' },
+ { number: 580, title: '0.58' },
+ { number: 581, title: '0.58.1' },
+ { number: 599, title: '0.59.9' },
+ { number: 5910, title: '0.59.10' },
+ { number: 600, title: '0.60' },
+ { number: 6010, title: '0.60.10' },
+ { number: 6111, title: '0.61.1.1' },
+ { number: 612, title: '0.61.2' },
+ { number: 613, title: '0.61.3'},
+ { number: 621, title: '0.62.1'},
+ { number: 6220, title: '0.62.20'},
+ { number: 62300, title: '0.62.300'},
+ { number: 630, title: '0.63'},
+ ] as Milestone[];
+
+ describe('getNextMilestone', () => {
+ it('can detect the next milestone for minor versions', () => {
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 57,
+ })?.number).toBe(577);
+
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 59,
+ })?.number).toBe(599);
+
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 62,
+ })?.number).toBe(621);
+ });
+
+ it('can detect the next milestone for major versions', () => {
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 63,
+ })?.number).toBe(630);
+ });
+
+ it('can detect the next milestone for mixed major and minor versions', () => {
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 58,
+ })?.number).toBe(580);
+
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 60,
+ })?.number).toBe(600);
+ });
+
+ it('should return undefined when milestone is missing', () => {
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 56,
+ })?.number).toBe(undefined);
+ });
+
+ it('should ignore patch versions', () => {
+ expect(getNextMilestone({
+ openMilestones: testMilestones,
+ majorVersion: 61,
+ })?.number).toBe(612);
+ });
+ });
+});
diff --git a/release/src/types.ts b/release/src/types.ts
index e655cdbaf2a..a3172403ac7 100644
--- a/release/src/types.ts
+++ b/release/src/types.ts
@@ -15,14 +15,16 @@ export interface CacheType {
slackChannelId?: string;
preReleaseActionId?: number;
}
-
-export interface ReleaseProps {
+export interface GithubProps {
owner: string;
repo: string;
- version: string;
github: Octokit;
}
+export interface ReleaseProps extends GithubProps {
+ version: string;
+}
+
export interface VersionInfo {
version: string;
released: string;
@@ -43,3 +45,15 @@ export type Issue = {
assignee: null | { login: string };
created_at: string;
};
+
+export type Milestone = {
+ url: string;
+ html_url: string;
+ labels_url: string;
+ id: number;
+ node_id: string;
+ number: number;
+ state: "open" | "closed";
+ title: string;
+ description: string | null;
+};
diff --git a/release/tsconfig.json b/release/tsconfig.json
index fe3c32e1c41..03fe48018aa 100644
--- a/release/tsconfig.json
+++ b/release/tsconfig.json
@@ -9,7 +9,8 @@
"experimentalDecorators": true,
"forceConsistentCasingInFileNames": true,
"lib": ["dom", "dom.iterable", "esnext"],
- "outDir": "./dist"
+ "outDir": "./dist",
+ "target": "esnext"
},
"include": ["src/**/*.ts", "src/index.mts"]
}
--
GitLab