Skip to content

Commit

Permalink
feat(ng-dev): support exceptional minor release train in merge targeting
Browse files Browse the repository at this point in the history
We are still exploring an alternative long-term solution with
a potentially simpler mental model, but for now we are going
with a minimal-invasive solution that allows for exceptional minors
to become an offically-supported situation.

If there is no exceptional minor, target: minor will behave like before
If there is an exceptional minor, target: patch always goes in there
If there is an exceptional minor, target: minor will only behave
  differently if the GitHub PR target is explicitly set to the exceptional minor.

So in TL:DR: Everything remains the same unless there is an exceptional
minor. But even then, targeting like before would work the same without
any required process/model change.
  • Loading branch information
devversion committed Dec 22, 2022
1 parent d5693db commit 12034dc
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 5 deletions.
19 changes: 17 additions & 2 deletions ng-dev/pr/common/targeting/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {targetLabels} from '../labels/target.js';
* NPM version data when LTS version branches are validated.
*/
export async function getTargetLabelConfigsForActiveReleaseTrains(
{latest, releaseCandidate, next}: ActiveReleaseTrains,
{latest, releaseCandidate, next, exceptionalMinor}: ActiveReleaseTrains,
api: GithubClient,
config: NgDevConfig<{
github: GithubConfig;
Expand Down Expand Up @@ -81,7 +81,15 @@ export async function getTargetLabelConfigsForActiveReleaseTrains(
},
{
label: targetLabels.TARGET_MINOR,
branches: () => {
branches: (githubTargetBranch) => {
// If there is an exceptional minor in-progress, and a PR specifically sets
// its destination to it, along with `target: minor`, then we merge into it.
// This allows for an exceptional minor train to receive e.g. features.
// See: http://go/angular-exceptional-minor
if (githubTargetBranch === exceptionalMinor?.branchName) {
return [exceptionalMinor.branchName];
}

return [nextBranchName];
},
},
Expand All @@ -102,6 +110,13 @@ export async function getTargetLabelConfigsForActiveReleaseTrains(
if (releaseCandidate !== null) {
branches.push(releaseCandidate.branchName);
}
// If there is an exceptional minor, patch changes should always go into it.
// It would be a potential loss of fixes/patches if suddenly the exceptional
// minor becomes the new patch- but misses some commits.
// More details here: http://go/angular-exceptional-minor.
if (exceptionalMinor !== null) {
branches.push(exceptionalMinor.branchName);
}
return branches;
},
},
Expand Down
103 changes: 100 additions & 3 deletions ng-dev/pr/merge/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import {
} from '../common/targeting/target-label.js';
import {fakeGithubPaginationResponse} from '../../utils/testing/github-interception.js';
import {getTargetLabelConfigsForActiveReleaseTrains} from '../common/targeting/labels.js';
import {ActiveReleaseTrains} from '../../release/versioning/index.js';
import {
ActiveReleaseTrains,
exceptionalMinorPackageIndicator,
PackageJson,
} from '../../release/versioning/index.js';
import {Prompt} from '../../utils/prompt.js';

const API_ENDPOINT = `https://api.github.com`;
Expand Down Expand Up @@ -53,11 +57,20 @@ describe('default target labels', () => {
* Mocks a branch `package.json` version API request.
* https://docs.github.com/en/rest/reference/repos#get-repository-content.
*/
function interceptBranchVersionRequest(branchName: string, version: string) {
function interceptBranchVersionRequest(
branchName: string,
version: string,
opts?: {exceptionalMinor: boolean},
) {
const pkgJson: PackageJson = {version};
if (opts?.exceptionalMinor) {
pkgJson[exceptionalMinorPackageIndicator] = true;
}

nock(getRepoApiRequestUrl())
.get('/contents/%2Fpackage.json')
.query((params) => params.ref === branchName)
.reply(200, {content: Buffer.from(JSON.stringify({version})).toString('base64')});
.reply(200, {content: Buffer.from(JSON.stringify(pkgJson)).toString('base64')});
}

/** Fakes a prompt confirm question with the given value. */
Expand Down Expand Up @@ -524,4 +537,88 @@ describe('default target labels', () => {
});
});
});

describe('with an in-progress exceptional minor', () => {
describe('major in FF/RC release-train', () => {
it('patch changes should always be included in the exceptional minor train', async () => {
interceptBranchVersionRequest('master', '13.1.0-next.0');
interceptBranchVersionRequest('13.0.x', '13.0.0-rc.3');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x', '13.0.x']);

expect(await getBranchesForLabel('target: patch')).toEqual([
'master',
'12.1.x',
'13.0.x',
'12.2.x',
]);
});

it('minor changes should still go into `master` if GitHub PR target is `main`', async () => {
interceptBranchVersionRequest('master', '13.1.0-next.0');
interceptBranchVersionRequest('13.0.x', '13.0.0-rc.3');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x', '13.0.x']);

expect(await getBranchesForLabel('target: minor')).toEqual(['master']);
});

it('minor changes can go into an exceptional minor if explicitly targeted', async () => {
interceptBranchVersionRequest('master', '13.1.0-next.0');
interceptBranchVersionRequest('13.0.x', '13.0.0-rc.3');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x', '13.0.x']);

expect(await getBranchesForLabel('target: minor', '12.2.x')).toEqual(['12.2.x']);
});

it('should be possible to target exceptional minor + rc + next trains via two PRs', async () => {
const installMocks = () => {
interceptBranchVersionRequest('master', '13.1.0-next.0');
interceptBranchVersionRequest('13.0.x', '13.0.0-rc.3');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x', '13.0.x']);
};

// First PR to land in FF/RC + main trains.
installMocks();
expect(await getBranchesForLabel('target: rc')).toEqual(['master', '13.0.x']);

// Second PR to explicitly backport a change into the exceptional minor.
installMocks();
expect(await getBranchesForLabel('target: minor', '12.2.x')).toEqual(['12.2.x']);
});
});

it('patch changes should always be included in the exceptional minor train', async () => {
interceptBranchVersionRequest('master', '13.0.0-next.0');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x']);

expect(await getBranchesForLabel('target: patch')).toEqual(['master', '12.1.x', '12.2.x']);
});

it('minor changes should still go into `master` if GitHub PR target is `main`', async () => {
interceptBranchVersionRequest('master', '13.0.0-next.0');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x']);

expect(await getBranchesForLabel('target: minor')).toEqual(['master']);
});

it('minor changes can go into an exceptional minor if explicitly targeted', async () => {
interceptBranchVersionRequest('master', '13.0.0-next.0');
interceptBranchVersionRequest('12.2.x', '12.2.0-next.0', {exceptionalMinor: true});
interceptBranchVersionRequest('12.1.x', '12.1.0');
interceptBranchesListRequest(['12.1.x', '12.2.x']);

expect(await getBranchesForLabel('target: minor', '12.2.x')).toEqual(['12.2.x']);
});
});
});

0 comments on commit 12034dc

Please sign in to comment.