From b7ecd63ffed487c93c0d1b7b10be5047e9124e49 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Thu, 8 Jun 2023 14:37:18 +0200 Subject: [PATCH 1/4] feat(backport): log determined target branches This is helpful to users to better understand which target branches have been determined to backport to. (cherry picked from commit a01196f23f22569577ae51369b18e3c378d1de7d) --- src/backport.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 9e662eb..d9ebb47 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -378,9 +378,13 @@ export function findTargetBranches( `Found target branches in \`target_branches\` input: ${configuredTargetBranches}` ); - return [ + const targetBranches = [ ...new Set([...targetBranchesFromLabels, ...configuredTargetBranches]), ]; + + console.log(`Determined target branches: ${targetBranches}`); + + return targetBranches; } function findTargetBranchesFromLabels( From 665ab084362b72f2f9fa7fbca798b4cbbb7c4178 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Thu, 8 Jun 2023 14:39:16 +0200 Subject: [PATCH 2/4] refactor: make headref available to find target branches If we want to exclude the headref from the possible target branches, we need to provide it as a parameter to the findTargetBranches function. I'm not so happy that this affects all previously written unit tests for this function, but it is acceptable because headref has now become important for this function. (cherry picked from commit e6e5284bc5d260df5f27d5f49e868a437e1a9912) --- src/backport.ts | 5 ++-- src/github.ts | 1 + src/test/backport.test.ts | 61 ++++++++++++++++++++++++--------------- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/backport.ts b/src/backport.ts index d9ebb47..8a6fee9 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -264,7 +264,7 @@ export class Backport { private findTargetBranches(mainpr: PullRequest, config: Config): string[] { const labels = mainpr.labels.map((label) => label.name); - return findTargetBranches(config, labels); + return findTargetBranches(config, labels, mainpr.head.ref); } private composePRContent(target: string, main: PullRequest): PRContent { @@ -360,7 +360,8 @@ export class Backport { export function findTargetBranches( config: Pick, - labels: string[] + labels: string[], + headref: string ) { console.log("Determining target branches..."); diff --git a/src/github.ts b/src/github.ts index f7b0d57..73ff67d 100644 --- a/src/github.ts +++ b/src/github.ts @@ -134,6 +134,7 @@ export type PullRequest = { body: string | null; head: { sha: string; + ref: string; }; base: { sha: string; diff --git a/src/test/backport.test.ts b/src/test/backport.test.ts index b7e4366..f2ee36a 100644 --- a/src/test/backport.test.ts +++ b/src/test/backport.test.ts @@ -10,33 +10,41 @@ describe("find target branches", () => { describe("returns an empty list", () => { it("when labels is an empty list", () => { expect( - findTargetBranches({ labels: { pattern: default_pattern } }, []) + findTargetBranches( + { labels: { pattern: default_pattern } }, + [], + "feature/one" + ) ).toEqual([]); }); it("when none of the labels match the pattern", () => { expect( - findTargetBranches({ labels: { pattern: default_pattern } }, [ - "a label", - "another-label", - "a/third/label", - ]) + findTargetBranches( + { labels: { pattern: default_pattern } }, + ["a label", "another-label", "a/third/label"], + "feature/one" + ) ).toEqual([]); }); it("when a label matches the pattern but doesn't capture a target branch", () => { expect( - findTargetBranches({ labels: { pattern: /^no capture group$/ } }, [ - "no capture group", - ]) + findTargetBranches( + { labels: { pattern: /^no capture group$/ } }, + ["no capture group"], + "feature/one" + ) ).toEqual([]); }); it("when the label pattern is an empty string", () => { expect( - findTargetBranches({ labels: { pattern: undefined } }, [ - "an empty string", - ]) + findTargetBranches( + { labels: { pattern: undefined } }, + ["an empty string"], + "feature/one" + ) ).toEqual([]); }); @@ -44,7 +52,8 @@ describe("find target branches", () => { expect( findTargetBranches( { labels: { pattern: default_pattern }, target_branches: "" }, - ["a label"] + ["a label"], + "feature/one" ) ).toEqual([]); }); @@ -53,18 +62,21 @@ describe("find target branches", () => { describe("returns selected branches", () => { it("when a label matches the pattern and captures a target branch", () => { expect( - findTargetBranches({ labels: { pattern: default_pattern } }, [ - "backport release-1", - ]) + findTargetBranches( + { labels: { pattern: default_pattern } }, + ["backport release-1"], + "feature/one" + ) ).toEqual(["release-1"]); }); it("when several labels match the pattern and capture a target branch", () => { expect( - findTargetBranches({ labels: { pattern: default_pattern } }, [ - "backport release-1", - "backport another/target/branch", - ]) + findTargetBranches( + { labels: { pattern: default_pattern } }, + ["backport release-1", "backport another/target/branch"], + "feature/one" + ) ).toEqual(["release-1", "another/target/branch"]); }); @@ -75,7 +87,8 @@ describe("find target branches", () => { labels: { pattern: default_pattern }, target_branches: "release-1", }, - [] + [], + "feature/one" ) ).toEqual(["release-1"]); }); @@ -87,7 +100,8 @@ describe("find target branches", () => { labels: { pattern: default_pattern }, target_branches: "release-1 another/target/branch", }, - [] + [], + "feature/one" ) ).toEqual(["release-1", "another/target/branch"]); }); @@ -99,7 +113,8 @@ describe("find target branches", () => { labels: { pattern: default_pattern }, target_branches: "release-1", }, - ["backport release-1"] + ["backport release-1"], + "feature/one" ) ).toEqual(["release-1"]); }); From 43df180f244090bcd1abde7464df80507505727c Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Thu, 8 Jun 2023 14:41:38 +0200 Subject: [PATCH 3/4] feat(backport): exclude headref from found target branches This case is relevant where the target_branches input is used to select the target branches, and the original pull request's headref is part of it. For example, consider selecting all feature branches `feature/*` as target_branches. Glob patterns are not supported yet, but users can find these with an additional step in their workflow: ``` run: | branches=$(git branch --list --all | grep 'origin/feature/' | cut -c 18- ) space_delimited=${branches//$'\n'/ } echo "BRANCHES=${space_delimited}" >> $GITHUB_OUTPUT ``` Note that such a step requires a deep checkout `depth=0`. But when users do this, they might encounter the feature branch they just merged, the merged branch that triggered the backport-action. By excluding it now, we can better support this case. In addition, it will help support glob patterns in the target_branches input in the future. (cherry picked from commit 151736de05fd9f0f9473618caa8842ba92cd48df) --- src/backport.ts | 5 ++++- src/test/backport.test.ts | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/backport.ts b/src/backport.ts index 8a6fee9..e65928d 100644 --- a/src/backport.ts +++ b/src/backport.ts @@ -378,10 +378,13 @@ export function findTargetBranches( console.log( `Found target branches in \`target_branches\` input: ${configuredTargetBranches}` ); + console.log( + `Exclude pull request's headref from target branches: ${headref}` + ); const targetBranches = [ ...new Set([...targetBranchesFromLabels, ...configuredTargetBranches]), - ]; + ].filter((t) => t !== headref); console.log(`Determined target branches: ${targetBranches}`); diff --git a/src/test/backport.test.ts b/src/test/backport.test.ts index f2ee36a..e39f266 100644 --- a/src/test/backport.test.ts +++ b/src/test/backport.test.ts @@ -57,6 +57,26 @@ describe("find target branches", () => { ) ).toEqual([]); }); + + it("when the label pattern only matches the headref", () => { + expect( + findTargetBranches( + { labels: { pattern: default_pattern } }, + ["backport feature/one"], + "feature/one" + ) + ).toEqual([]); + }); + + it("when target_branches only contains the headref", () => { + expect( + findTargetBranches( + { labels: {}, target_branches: "feature/one" }, + [], + "feature/one" + ) + ).toEqual([]); + }); }); describe("returns selected branches", () => { @@ -118,5 +138,25 @@ describe("find target branches", () => { ) ).toEqual(["release-1"]); }); + + it("when several labels match the pattern the headref is excluded", () => { + expect( + findTargetBranches( + { labels: { pattern: default_pattern } }, + ["backport feature/one", "backport feature/two"], + "feature/one" + ) + ).toEqual(["feature/two"]); + }); + + it("when several target branches are specified the headref is excluded", () => { + expect( + findTargetBranches( + { labels: {}, target_branches: "feature/one feature/two" }, + [], + "feature/one" + ) + ).toEqual(["feature/two"]); + }); }); }); From 603de4023d56039e8683746d6cccd3f114720021 Mon Sep 17 00:00:00 2001 From: Nico Korthout Date: Thu, 8 Jun 2023 14:53:19 +0200 Subject: [PATCH 4/4] docs: exclude headref from selected target branches The headref is excluded automatically. Since this is an explicit difference in behavior, it should be clearly documented to avoid surprises for users. As far as I know, there is no use case to backport a pull request to its headref. The cherry-pick would be interrupted because there are no changes which would result in empty commits. Typically, the git user get's to decide to skip these commits, or to abort the cherry-pick. This means that would not be any new commits, and we cannot open an empty pull request. So all of this doesn't work. The only thing left to do, is exclude the headref from the determined target branches. (cherry picked from commit 2f0f6c9f2c77723fc0babea4a0dc31b904676a9d) --- README.md | 2 ++ action.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/README.md b/README.md index f1dacee..0b265a0 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ Must contain a capture group for the target branch. Label matching can be disabled entirely using an empty string `''` as pattern. The action will backport the pull request to each matched target branch. +Note that the pull request's headref is excluded automatically. See [How it works](#how-it-works). ### `pull_description` @@ -168,6 +169,7 @@ Please refer to this action's README for all available [placeholders](#placehold Default: `''` (disabled) The action will backport the pull request to each specified target branch (space-delimited). +Note that the pull request's headref is excluded automatically. See [How it works](#how-it-works). Can be used in addition to backport labels. diff --git a/action.yml b/action.yml index 6631fea..89a1f2f 100644 --- a/action.yml +++ b/action.yml @@ -22,6 +22,7 @@ inputs: Regex pattern to match the backport labels on the merged pull request. Must contain a capture group for the target branch. The action will backport the pull request to each matched target branch. + Note that the pull request's headref is excluded automatically. default: ^backport ([^ ]+)$ pull_description: description: > @@ -43,6 +44,7 @@ inputs: target_branches: description: > The action will backport the pull request to each specified target branch (space-delimited). + 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. outputs: