From 0283b4db12e69320797b31f7a50a5a1f2f567e2a Mon Sep 17 00:00:00 2001 From: Joshua Schmid Date: Thu, 2 Nov 2023 15:04:15 +0100 Subject: [PATCH 1/7] feat: add support for milestone, assignees and reviewers Signed-off-by: Joshua Schmid --- README.md | 22 +++++++++++++++++++- action.yml | 12 +++++++++++ src/backport.ts | 54 ++++++++++++++++++++++++++++++++++++++++++++++++- src/github.ts | 37 +++++++++++++++++++++++++++++++++ src/main.ts | 6 ++++++ 5 files changed, 129 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index aa8b900..d0918ad 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ jobs: runs-on: ubuntu-latest # Only run when pull request is merged - # or when a comment containing `/backport` is created by someone other than the + # or when a comment containing `/backport` is created by someone other than the # https://github.com/backport-action bot user (user id: 97796249). Note that if you use your # own PAT as `github_token`, that you should replace this id with yours. if: > @@ -185,6 +185,26 @@ See [How it works](#how-it-works). Can be used in addition to backport labels. By default, only backport labels are used to specify the target branches. + +### `copy_milestone` + +Default: `''` (disabled) + +Controls wheather the "milestone" of the original pull request should be added to the backports. + +### `copy_assignees` + +Default: `''` (disabled) + +Controls wheather the assignees of the original pull request should be added to the backports. + +### `copy_reviewers` + +Default: `''` (disabled) + +Controls wheather the reviewers of the original pull request should be added to the backports. + + ## Placeholders In the `pull_description` and `pull_title` inputs, placeholders can be used to define variable values. These are indicated by a dollar sign and curly braces (`${placeholder}`). diff --git a/action.yml b/action.yml index a7dd497..4599bb2 100644 --- a/action.yml +++ b/action.yml @@ -53,6 +53,18 @@ inputs: Note that the pull request's headref is excluded automatically. Can be used in addition to backport labels. By default, only backport labels are used to specify the target branches. + copy_milestone: + description: > + Whether or not to copy the milestone from the original pull request to the backport pull request. + By default, the milestone is not copied. + copy_assignees: + description: > + Whether or not to copy the assignees from the original pull request to the backport pull request. + By default, the assignees are not copied. + copy_reviewers: + description: > + Whether or not to copy the reviewers from the original pull request to the backport pull request. + By default, the reviewers are not copied. outputs: was_successful: description: > diff --git a/src/backport.ts b/src/backport.ts index 013baa6..9a227cf 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -25,6 +25,9 @@ export type Config = { commits: { merge_commits: "fail" | "skip"; }; + copy_milestone: boolean; + copy_assignees: boolean; + copy_reviewers: boolean; }; enum Output { @@ -257,6 +260,54 @@ export class Backport { } const new_pr = new_pr_response.data; + if (this.config.copy_milestone == true) { + const milestone = mainpr.milestone?.number; + if (milestone) { + console.info("Setting milestone to " + milestone); + const set_milestone_response = await this.github.setMilestone( + new_pr.number, + milestone, + ); + if (set_milestone_response.status != 200) { + console.error(set_milestone_response.status); + console.error(JSON.stringify(set_milestone_response)); + } + } + } + + if (this.config.copy_assignees == true) { + const assignees = mainpr.assignees.map((label) => label.login); + if (assignees.length > 0) { + console.info("Setting assignees " + assignees); + const set_assignee_response = await this.github.setAssignees( + new_pr.number, + assignees, + ); + if (set_assignee_response.status != 201) { + console.error(set_assignee_response.status); + } + } + } + + if (this.config.copy_reviewers == true) { + const reviewers = mainpr.requested_reviewers?.map( + (reviewer) => reviewer.login, + ); + if (reviewers?.length > 0) { + console.info("Setting reviewers " + reviewers); + const reviewRequest = { + ...this.github.getRepo(), + pull_number: new_pr.number, + reviewers: reviewers, + }; + const set_reviewers_response = + await this.github.requestReviewers(reviewRequest); + if (set_reviewers_response.status != 201) { + console.error(set_reviewers_response.status); + } + } + } + if (labelsToCopy.length > 0) { const label_response = await this.github.labelPR( new_pr.number, @@ -306,6 +357,7 @@ export class Backport { } } + private findTargetBranches(mainpr: PullRequest, config: Config): string[] { const labels = mainpr.labels.map((label) => label.name); return findTargetBranches(config, labels, mainpr.head.ref); @@ -377,7 +429,7 @@ export class Backport { private composeMessageForCreatePRFailed( response: CreatePullRequestResponse, ): string { - return dedent`Backport branch created but failed to create PR. + return dedent`Backport branch created but failed to create PR. Request to create PR rejected with status ${response.status}. (see action log for full response)`; diff --git a/src/github.ts b/src/github.ts index 612ee4d..7b54b2d 100644 --- a/src/github.ts +++ b/src/github.ts @@ -20,6 +20,8 @@ export interface GithubApi { createPR(pr: CreatePullRequest): Promise; labelPR(pr: number, labels: string[]): Promise; requestReviewers(request: ReviewRequest): Promise; + setAssignees(pr: number, assignees: string[]): Promise; + setMilestone(pr: number, milestone: number): Promise; } export class Github implements GithubApi { @@ -126,6 +128,24 @@ export class Github implements GithubApi { labels, }); } + + public async setAssignees(pr: number, assignees: string[]) { + console.log(`Set Assignees ${assignees} to #${pr}`); + return this.#octokit.rest.issues.addAssignees({ + ...this.getRepo(), + issue_number: pr, + assignees, + }); + } + + public async setMilestone(pr: number, milestone: number) { + console.log(`Set Milestone ${milestone} to #${pr}`); + return this.#octokit.rest.issues.update({ + ...this.getRepo(), + issue_number: pr, + milestone: milestone, + }); + } } export type PullRequest = { @@ -149,6 +169,18 @@ export type PullRequest = { login: string; }[]; commits: number; + milestone: { + number: number; + id: number; + title: string; + }; + assignees: { + login: string; + id: number; + }[]; + merged_by: { + login: string; + }; }; export type CreatePullRequestResponse = { status: number; @@ -158,6 +190,11 @@ export type CreatePullRequestResponse = { }; }; export type RequestReviewersResponse = CreatePullRequestResponse; + +export type GenericResponse = { + status: number; +}; + export type LabelPullRequestResponse = { status: number; }; diff --git a/src/main.ts b/src/main.ts index b665fd0..2fcf8d7 100644 --- a/src/main.ts +++ b/src/main.ts @@ -18,6 +18,9 @@ async function run(): Promise { const copy_labels_pattern = core.getInput("copy_labels_pattern"); const target_branches = core.getInput("target_branches"); const merge_commits = core.getInput("merge_commits"); + const copy_assignees = core.getInput("copy_assignees"); + const copy_milestone = core.getInput("copy_milestone"); + const copy_reviewers = core.getInput("copy_reviewers"); if (merge_commits != "fail" && merge_commits != "skip") { const message = `Expected input 'merge_commits' to be either 'fail' or 'skip', but was '${merge_commits}'`; @@ -36,6 +39,9 @@ async function run(): Promise { copy_labels_pattern === "" ? undefined : new RegExp(copy_labels_pattern), target_branches: target_branches === "" ? undefined : target_branches, commits: { merge_commits }, + copy_assignees: copy_assignees === "true", + copy_milestone: copy_milestone === "true", + copy_reviewers: copy_reviewers === "true", }; const backport = new Backport(github, config, git); From 364fd879b22c4e31d5e44145f4898d6d485f38d2 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 13:41:08 +0100 Subject: [PATCH 2/7] docs: sort inputs lexicographically Whether this is the best ordering is subjective, but I prefer a specific ordering like this until I see a better one (perhaps by categories or so). --- README.md | 38 ++++++++++++++++++-------------------- action.yml | 24 ++++++++++++------------ 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index d0918ad..4517b01 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,12 @@ jobs: The action can be configured with the following optional [inputs](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepswith): +### `copy_assignees` + +Default: `''` (disabled) + +Controls wheather the assignees of the original pull request should be added to the backports. + ### `copy_labels_pattern` Default: `''` (disabled) @@ -113,6 +119,18 @@ Regex pattern to match github labels which will be copied from the original pull Note that labels matching `label_pattern` are excluded. By default, no labels are copied. +### `copy_milestone` + +Default: `''` (disabled) + +Controls wheather the "milestone" of the original pull request should be added to the backports. + +### `copy_reviewers` + +Default: `''` (disabled) + +Controls wheather the reviewers of the original pull request should be added to the backports. + ### `github_token` Default: `${{ github.token }}` @@ -185,26 +203,6 @@ See [How it works](#how-it-works). Can be used in addition to backport labels. By default, only backport labels are used to specify the target branches. - -### `copy_milestone` - -Default: `''` (disabled) - -Controls wheather the "milestone" of the original pull request should be added to the backports. - -### `copy_assignees` - -Default: `''` (disabled) - -Controls wheather the assignees of the original pull request should be added to the backports. - -### `copy_reviewers` - -Default: `''` (disabled) - -Controls wheather the reviewers of the original pull request should be added to the backports. - - ## Placeholders In the `pull_description` and `pull_title` inputs, placeholders can be used to define variable values. These are indicated by a dollar sign and curly braces (`${placeholder}`). diff --git a/action.yml b/action.yml index 4599bb2..6f9a945 100644 --- a/action.yml +++ b/action.yml @@ -3,11 +3,23 @@ description: > Fast and flexible action to cherry-pick commits from labeled pull requests author: korthout inputs: + copy_assignees: + description: > + Whether or not to copy the assignees from the original pull request to the backport pull request. + By default, the assignees are not copied. copy_labels_pattern: description: > Regex pattern to match github labels which will be copied from the original pull request to the backport pull request. Note that labels matching `label_pattern` are excluded. By default, no labels are copied. + copy_milestone: + description: > + Whether or not to copy the milestone from the original pull request to the backport pull request. + By default, the milestone is not copied. + copy_reviewers: + description: > + Whether or not to copy the reviewers from the original pull request to the backport pull request. + By default, the reviewers are not copied. github_token: description: > Token to authenticate requests to GitHub. @@ -53,18 +65,6 @@ inputs: Note that the pull request's headref is excluded automatically. Can be used in addition to backport labels. By default, only backport labels are used to specify the target branches. - copy_milestone: - description: > - Whether or not to copy the milestone from the original pull request to the backport pull request. - By default, the milestone is not copied. - copy_assignees: - description: > - Whether or not to copy the assignees from the original pull request to the backport pull request. - By default, the assignees are not copied. - copy_reviewers: - description: > - Whether or not to copy the reviewers from the original pull request to the backport pull request. - By default, the reviewers are not copied. outputs: was_successful: description: > From 397d7f21bdb6faa9236edd2524d76088c24b3726 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 13:46:59 +0100 Subject: [PATCH 3/7] docs: specify false as default for new inputs The inputs expect `true` to enable the copying behavior. It wasn't really clear from the action.yml nor from the readme that these are boolean flags. By specifying the default as `false` this becomes clear. Ultimately, an empty input would still disable the copying behavior, but that's less relevant than the knowledge that this is a boolean flag. This change doesn't affect the usage or implementation. --- README.md | 6 +++--- action.yml | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4517b01..f1641a5 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ The action can be configured with the following optional [inputs](https://docs.g ### `copy_assignees` -Default: `''` (disabled) +Default: `false` (disabled) Controls wheather the assignees of the original pull request should be added to the backports. @@ -121,13 +121,13 @@ By default, no labels are copied. ### `copy_milestone` -Default: `''` (disabled) +Default: `false` (disabled) Controls wheather the "milestone" of the original pull request should be added to the backports. ### `copy_reviewers` -Default: `''` (disabled) +Default: `false` (disabled) Controls wheather the reviewers of the original pull request should be added to the backports. diff --git a/action.yml b/action.yml index 6f9a945..66ed62f 100644 --- a/action.yml +++ b/action.yml @@ -7,6 +7,7 @@ inputs: description: > Whether or not to copy the assignees from the original pull request to the backport pull request. By default, the assignees are not copied. + default: false copy_labels_pattern: description: > Regex pattern to match github labels which will be copied from the original pull request to the backport pull request. @@ -16,10 +17,12 @@ inputs: description: > Whether or not to copy the milestone from the original pull request to the backport pull request. By default, the milestone is not copied. + default: false copy_reviewers: description: > Whether or not to copy the reviewers from the original pull request to the backport pull request. By default, the reviewers are not copied. + default: false github_token: description: > Token to authenticate requests to GitHub. From fe78391473d575e6193777b453c44af3deab04d3 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 13:56:51 +0100 Subject: [PATCH 4/7] docs: align the input wording I'd like to keep the descriptions of the inputs aligned where possible/useful to avoid confusion about what they do. --- README.md | 9 ++++++--- action.yml | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index f1641a5..ff56f3c 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,8 @@ The action can be configured with the following optional [inputs](https://docs.g Default: `false` (disabled) -Controls wheather the assignees of the original pull request should be added to the backports. +Controls whether to copy the assignees from the original pull request to the backport pull request. +By default, the assignees are not copied. ### `copy_labels_pattern` @@ -123,13 +124,15 @@ By default, no labels are copied. Default: `false` (disabled) -Controls wheather the "milestone" of the original pull request should be added to the backports. +Controls whether to copy the milestone from the original pull request to the backport pull request. +By default, the milestone is not copied. ### `copy_reviewers` Default: `false` (disabled) -Controls wheather the reviewers of the original pull request should be added to the backports. +Controls whether to copy the reviewers from the original pull request to the backport pull request. +By default, the reviewers are not copied. ### `github_token` diff --git a/action.yml b/action.yml index 66ed62f..18f46a8 100644 --- a/action.yml +++ b/action.yml @@ -5,7 +5,7 @@ author: korthout inputs: copy_assignees: description: > - Whether or not to copy the assignees from the original pull request to the backport pull request. + Controls whether to copy the assignees from the original pull request to the backport pull request. By default, the assignees are not copied. default: false copy_labels_pattern: @@ -15,12 +15,12 @@ inputs: By default, no labels are copied. copy_milestone: description: > - Whether or not to copy the milestone from the original pull request to the backport pull request. + Controls whether to copy the milestone from the original pull request to the backport pull request. By default, the milestone is not copied. default: false copy_reviewers: description: > - Whether or not to copy the reviewers from the original pull request to the backport pull request. + Controls whether to copy the reviewers from the original pull request to the backport pull request. By default, the reviewers are not copied. default: false github_token: From 45f65af739fd8b3088300ab38f481063830f969c Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 13:59:56 +0100 Subject: [PATCH 5/7] fix: copy requested reviewers not reviewers The current implementation is not aligned with this description. It copies the currently open review requests and does not request reviews from those that already reviewed the original pull request. Let's adjust the name and the description to fit what it does: copy the requested reviewers not the actual reviewers. --- README.md | 7 ++++--- action.yml | 7 ++++--- src/backport.ts | 4 ++-- src/main.ts | 4 ++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index ff56f3c..65891be 100644 --- a/README.md +++ b/README.md @@ -127,12 +127,13 @@ Default: `false` (disabled) Controls whether to copy the milestone from the original pull request to the backport pull request. By default, the milestone is not copied. -### `copy_reviewers` +### `copy_requested_reviewers` Default: `false` (disabled) -Controls whether to copy the reviewers from the original pull request to the backport pull request. -By default, the reviewers are not copied. +Controls whether to copy the requested reviewers from the original pull request to the backport pull request. +Note that this does not request reviews from those users who already reviewed the original pull request. +By default, the requested reviewers are not copied. ### `github_token` diff --git a/action.yml b/action.yml index 18f46a8..8690342 100644 --- a/action.yml +++ b/action.yml @@ -18,10 +18,11 @@ inputs: Controls whether to copy the milestone from the original pull request to the backport pull request. By default, the milestone is not copied. default: false - copy_reviewers: + copy_requested_reviewers: description: > - Controls whether to copy the reviewers from the original pull request to the backport pull request. - By default, the reviewers are not copied. + Controls whether to copy the requested reviewers from the original pull request to the backport pull request. + Note that this does not request reviews from those users who already reviewed the original pull request. + By default, the requested reviewers are not copied. default: false github_token: description: > diff --git a/src/backport.ts b/src/backport.ts index 9a227cf..67a8839 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -27,7 +27,7 @@ export type Config = { }; copy_milestone: boolean; copy_assignees: boolean; - copy_reviewers: boolean; + copy_requested_reviewers: boolean; }; enum Output { @@ -289,7 +289,7 @@ export class Backport { } } - if (this.config.copy_reviewers == true) { + if (this.config.copy_requested_reviewers == true) { const reviewers = mainpr.requested_reviewers?.map( (reviewer) => reviewer.login, ); diff --git a/src/main.ts b/src/main.ts index 2fcf8d7..618f51d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -20,7 +20,7 @@ async function run(): Promise { const merge_commits = core.getInput("merge_commits"); const copy_assignees = core.getInput("copy_assignees"); const copy_milestone = core.getInput("copy_milestone"); - const copy_reviewers = core.getInput("copy_reviewers"); + const copy_requested_reviewers = core.getInput("copy_requested_reviewers"); if (merge_commits != "fail" && merge_commits != "skip") { const message = `Expected input 'merge_commits' to be either 'fail' or 'skip', but was '${merge_commits}'`; @@ -41,7 +41,7 @@ async function run(): Promise { commits: { merge_commits }, copy_assignees: copy_assignees === "true", copy_milestone: copy_milestone === "true", - copy_reviewers: copy_reviewers === "true", + copy_requested_reviewers: copy_requested_reviewers === "true", }; const backport = new Backport(github, config, git); From 51ff34ebce5ffb9d045e029f00ef19c346297018 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 14:04:31 +0100 Subject: [PATCH 6/7] fix: align error logs We can log the entire response for any of the failed requests. They may contain valuable info. --- src/backport.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backport.ts b/src/backport.ts index 67a8839..3bc1854 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -269,7 +269,6 @@ export class Backport { milestone, ); if (set_milestone_response.status != 200) { - console.error(set_milestone_response.status); console.error(JSON.stringify(set_milestone_response)); } } @@ -284,7 +283,7 @@ export class Backport { assignees, ); if (set_assignee_response.status != 201) { - console.error(set_assignee_response.status); + console.error(JSON.stringify(set_assignee_response)); } } } @@ -303,7 +302,7 @@ export class Backport { const set_reviewers_response = await this.github.requestReviewers(reviewRequest); if (set_reviewers_response.status != 201) { - console.error(set_reviewers_response.status); + console.error(JSON.stringify(set_reviewers_response)); } } } From 185de069d2405281d4249cf55cb26d67593e849b Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Sun, 5 Nov 2023 14:13:26 +0100 Subject: [PATCH 7/7] style: apply formatting --- src/backport.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 3bc1854..ec395cc 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -356,7 +356,6 @@ export class Backport { } } - private findTargetBranches(mainpr: PullRequest, config: Config): string[] { const labels = mainpr.labels.map((label) => label.name); return findTargetBranches(config, labels, mainpr.head.ref);