Skip to content

Commit ebc0e46

Browse files
devversionjosephperrott
authored andcommitted
refactor(dev-infra): improve error message for unexpected version branches (angular#38622)
Currently the merge script default branch configuration throws an error if an unexpected version branch is discovered. The error right now assumes to much knowledge of the logic and the document outlining the release trains conceptually. We change it to something more easy to understand that doesn't require full understanding of the versioning/labeling/branching document that has been created for the Angular organization. PR Close angular#38622
1 parent 3487b54 commit ebc0e46

File tree

2 files changed

+36
-11
lines changed

2 files changed

+36
-11
lines changed

dev-infra/pr/merge/defaults/branches.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ export async function findActiveVersionBranches(
165165
latestVersionBranch: string | null,
166166
releaseCandidateBranch: string | null,
167167
}> {
168+
// Version representing the release-train currently in the next phase. Note that we ignore
169+
// patch and pre-release segments in order to be able to compare the next release train to
170+
// other release trains from version branches (which follow the `N.N.x` pattern).
171+
const nextReleaseTrainVersion = semver.parse(`${nextVersion.major}.${nextVersion.minor}.0`)!;
172+
168173
let latestVersionBranch: string|null = null;
169174
let releaseCandidateBranch: string|null = null;
170175

@@ -177,15 +182,21 @@ export async function findActiveVersionBranches(
177182
// next version-branch as that one is supposed to be the latest active version-branch. If it
178183
// is not, then an error will be thrown due to two FF/RC branches existing at the same time.
179184
for (const {name, parsed} of branches) {
180-
// It can happen that version branches that are more recent than the version in the next
181-
// branch (i.e. `master`) have been created. We could ignore such branches silently, but
182-
// it might actually be symptomatic for an outdated version in the `next` branch, or an
185+
// It can happen that version branches have been accidentally created which are more recent
186+
// than the release-train in the next branch (i.e. `master`). We could ignore such branches
187+
// silently, but it might be symptomatic for an outdated version in the `next` branch, or an
183188
// accidentally created branch by the caretaker. In either way we want to raise awareness.
184-
if (semver.gte(parsed, nextVersion)) {
189+
if (semver.gt(parsed, nextReleaseTrainVersion)) {
190+
throw Error(
191+
`Discovered unexpected version-branch "${name}" for a release-train that is ` +
192+
`more recent than the release-train currently in the "${nextBranchName}" branch. ` +
193+
`Please either delete the branch if created by accident, or update the outdated ` +
194+
`version in the next branch (${nextBranchName}).`);
195+
} else if (semver.eq(parsed, nextReleaseTrainVersion)) {
185196
throw Error(
186-
`Discovered unexpected version-branch that is representing a minor ` +
187-
`version more recent than the one in the "${nextBranchName}" branch. Consider ` +
188-
`deleting the branch, or check if the version in "${nextBranchName}" is outdated.`);
197+
`Discovered unexpected version-branch "${name}" for a release-train that is already ` +
198+
`active in the "${nextBranchName}" branch. Please either delete the branch if ` +
199+
`created by accident, or update the version in the next branch (${nextBranchName}).`);
189200
}
190201

191202
const version = await getVersionOfBranch(repo, name);

dev-infra/pr/merge/defaults/integration.spec.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,17 +280,31 @@ describe('default target labels', () => {
280280
.toBeRejectedWithError('Invalid version detected in following branch: 11.1.x.');
281281
});
282282

283-
it('should error if branch more recent than version in "next" branch is found', async () => {
283+
it('should error if version-branch more recent than "next" is discovered', async () => {
284+
interceptBranchVersionRequest('master', '11.2.0-next.0');
285+
interceptBranchVersionRequest('11.3.x', '11.3.0-next.0');
286+
interceptBranchVersionRequest('11.1.x', '11.1.5');
287+
interceptBranchesListRequest(['11.1.x', '11.3.x']);
288+
289+
await expectAsync(getBranchesForLabel('target: lts', '10.2.x'))
290+
.toBeRejectedWithError(
291+
'Discovered unexpected version-branch "11.3.x" for a release-train that is ' +
292+
'more recent than the release-train currently in the "master" branch. Please ' +
293+
'either delete the branch if created by accident, or update the outdated version ' +
294+
'in the next branch (master).');
295+
});
296+
297+
it('should error if branch is matching with release-train in the "next" branch', async () => {
284298
interceptBranchVersionRequest('master', '11.2.0-next.0');
285299
interceptBranchVersionRequest('11.2.x', '11.2.0-next.0');
286300
interceptBranchVersionRequest('11.1.x', '11.1.5');
287301
interceptBranchesListRequest(['11.1.x', '11.2.x']);
288302

289303
await expectAsync(getBranchesForLabel('target: lts', '10.2.x'))
290304
.toBeRejectedWithError(
291-
'Discovered unexpected version-branch that is representing a minor version more ' +
292-
'recent than the one in the "master" branch. Consider deleting the branch, or check ' +
293-
'if the version in "master" is outdated.');
305+
'Discovered unexpected version-branch "11.2.x" for a release-train that is already ' +
306+
'active in the "master" branch. Please either delete the branch if created by ' +
307+
'accident, or update the version in the next branch (master).');
294308
});
295309

296310
it('should allow merging PR only into patch branch with "target: patch"', async () => {

0 commit comments

Comments
 (0)