Skip to content

Commit 6947cea

Browse files
gkalpakatscott
authored andcommitted
refactor(dev-infra): several code style and typo fixes (angular#39135)
This commit addresses comments from [my review][1] on PR angular#38656 (which was merged without comments addressed). The changes are mostly related to code style and typos. [1]: angular#38656 (review) PR Close angular#39135
1 parent 5471789 commit 6947cea

23 files changed

+73
-82
lines changed

.ng-dev/release.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const release: ReleaseConfig = {
2626
],
2727
// TODO: Implement release package building here.
2828
buildPackages: async () => [],
29-
// TODO: This can be removed once there is a org-wide tool for changelog generation.
29+
// TODO: This can be removed once there is an org-wide tool for changelog generation.
3030
generateReleaseNotesForHead: async () => {
3131
exec('yarn -s gulp changelog', {cwd: join(__dirname, '../')});
3232
},

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ export async function getDefaultTargetLabelConfiguration(
4141
// allow merging of PRs with `target: major`.
4242
if (!next.isMajor) {
4343
throw new InvalidTargetLabelError(
44-
`Unable to merge pull request. The "${nextBranchName}" branch will be ` +
45-
`released as a minor version.`);
44+
`Unable to merge pull request. The "${nextBranchName}" branch will be released as ` +
45+
'a minor version.');
4646
}
4747
return [nextBranchName];
4848
},

dev-infra/pr/merge/defaults/lts-branch.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export async function assertActiveLtsBranch(
3030
const ltsNpmTag = getLtsNpmDistTagOfMajor(version.major);
3131
const ltsVersion = semver.parse(distTags[ltsNpmTag]);
3232

33-
// Ensure that there is a LTS version tagged for the given version-branch major. e.g.
34-
// if the version branch is `9.2.x` then we want to make sure that there is a LTS
33+
// Ensure that there is an LTS version tagged for the given version-branch major. e.g.
34+
// if the version branch is `9.2.x` then we want to make sure that there is an LTS
3535
// version tagged in NPM for `v9`, following the `v{major}-lts` tag convention.
3636
if (ltsVersion === null) {
3737
throw new InvalidTargetBranchError(`No LTS version tagged for v${version.major} in NPM.`);

dev-infra/pr/merge/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export async function mergePullRequest(
3333
prNumber: number, githubToken: string, projectRoot: string = getRepoBaseDir(),
3434
config?: MergeConfigWithRemote) {
3535
// Set the environment variable to skip all git commit hooks triggered by husky. We are unable to
36-
// rely on `---no-verify` as some hooks still run, notably the `prepare-commit-msg` hook.
36+
// rely on `--no-verify` as some hooks still run, notably the `prepare-commit-msg` hook.
3737
process.env['HUSKY_SKIP_HOOKS'] = '1';
3838

3939
const api = await createPullRequestMergeTask(githubToken, projectRoot, config);

dev-infra/release/build/build.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ describe('ng-dev release build', () => {
6060
});
6161

6262
it('should error if package has not been built', async () => {
63-
// Set up a NPM package that is not built.
63+
// Set up an NPM package that is not built.
6464
npmPackages.push('@angular/non-existent');
6565

6666
spyOn(console, 'error');

dev-infra/release/publish/actions.ts

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {promises as fs} from 'fs';
10-
import * as Ora from 'ora';
10+
import * as ora from 'ora';
1111
import {join} from 'path';
1212
import * as semver from 'semver';
1313

@@ -44,7 +44,7 @@ export interface PullRequest {
4444
forkBranch: string;
4545
}
4646

47-
/** Constructor type for a instantiating a release action */
47+
/** Constructor type for instantiating a release action */
4848
export interface ReleaseActionConstructor<T extends ReleaseAction = ReleaseAction> {
4949
/** Whether the release action is currently active. */
5050
isActive(active: ActiveReleaseTrains): Promise<boolean>;
@@ -107,25 +107,22 @@ export abstract class ReleaseAction {
107107
if (state === 'failure') {
108108
error(
109109
red(` ✘ Cannot stage release. Commit "${commitSha}" does not pass all github ` +
110-
`status checks. Please make sure this commit passes all checks before re-running.`));
110+
'status checks. Please make sure this commit passes all checks before re-running.'));
111111
error(` Please have a look at: ${branchCommitsUrl}`);
112112

113113
if (await promptConfirm('Do you want to ignore the Github status and proceed?')) {
114114
info(yellow(
115-
` ⚠ Upstream commit is failing CI checks, but status has been ` +
116-
`forcibly ignored.`));
115+
' ⚠ Upstream commit is failing CI checks, but status has been forcibly ignored.'));
117116
return;
118117
}
119118
throw new UserAbortedReleaseActionError();
120119
} else if (state === 'pending') {
121120
error(
122121
red(` ✘ Commit "${commitSha}" still has pending github statuses that ` +
123-
`need to succeed before staging a release.`));
122+
'need to succeed before staging a release.'));
124123
error(red(` Please have a look at: ${branchCommitsUrl}`));
125124
if (await promptConfirm('Do you want to ignore the Github status and proceed?')) {
126-
info(yellow(
127-
` ⚠ Upstream commit is pending CI, but status has been ` +
128-
`forcibly ignored.`));
125+
info(yellow(' ⚠ Upstream commit is pending CI, but status has been forcibly ignored.'));
129126
return;
130127
}
131128
throw new UserAbortedReleaseActionError();
@@ -157,9 +154,9 @@ export abstract class ReleaseAction {
157154
*/
158155
protected async waitForEditsAndCreateReleaseCommit(newVersion: semver.SemVer) {
159156
info(yellow(
160-
` ⚠ Please review the changelog and ensure that the log contains only changes ` +
161-
`that apply to the public API surface. Manual changes can be made. When done, please ` +
162-
`proceed with the prompt below.`));
157+
' ⚠ Please review the changelog and ensure that the log contains only changes ' +
158+
'that apply to the public API surface. Manual changes can be made. When done, please ' +
159+
'proceed with the prompt below.'));
163160

164161
if (!await promptConfirm('Do you want to proceed and commit the changes?')) {
165162
throw new UserAbortedReleaseActionError();
@@ -188,7 +185,7 @@ export abstract class ReleaseAction {
188185
const forks = result.repository.forks.nodes;
189186

190187
if (forks.length === 0) {
191-
error(red(` ✘ Unable to find fork for currently authenticated user.`));
188+
error(red(' ✘ Unable to find fork for currently authenticated user.'));
192189
error(red(` Please ensure you created a fork of: ${owner}/${name}.`));
193190
throw new FatalReleaseActionError();
194191
}
@@ -304,7 +301,7 @@ export abstract class ReleaseAction {
304301
return new Promise((resolve, reject) => {
305302
debug(`Waiting for pull request #${id} to be merged.`);
306303

307-
const spinner = Ora().start(`Waiting for pull request #${id} to be merged.`);
304+
const spinner = ora().start(`Waiting for pull request #${id} to be merged.`);
308305
const intervalId = setInterval(async () => {
309306
const prState = await getPullRequestState(this.git, id);
310307
if (prState === 'merged') {
@@ -451,7 +448,7 @@ export abstract class ReleaseAction {
451448

452449
info(green(
453450
` ✓ Pull request for cherry-picking the changelog into "${nextBranch}" ` +
454-
`has been created.`));
451+
'has been created.'));
455452
info(yellow(` Please ask team members to review: ${url}.`));
456453
return true;
457454
}
@@ -490,7 +487,7 @@ export abstract class ReleaseAction {
490487

491488
if (!await this._isCommitForVersionStaging(newVersion, versionBumpCommitSha)) {
492489
error(red(` ✘ Latest commit in "${publishBranch}" branch is not a staging commit.`));
493-
error(red(` Please make sure the staging pull request has been merged.`));
490+
error(red(' Please make sure the staging pull request has been merged.'));
494491
throw new FatalReleaseActionError();
495492
}
496493

@@ -514,13 +511,13 @@ export abstract class ReleaseAction {
514511
await this._publishBuiltPackageToNpm(builtPackage, npmDistTag);
515512
}
516513

517-
info(green(` ✓ Published all packages successfully`));
514+
info(green(' ✓ Published all packages successfully'));
518515
}
519516

520517
/** Publishes the given built package to NPM with the specified NPM dist tag. */
521518
private async _publishBuiltPackageToNpm(pkg: BuiltPackage, npmDistTag: string) {
522519
debug(`Starting publish of "${pkg.name}".`);
523-
const spinner = Ora().start(`Publishing "${pkg.name}"`);
520+
const spinner = ora().start(`Publishing "${pkg.name}"`);
524521

525522
try {
526523
await runNpmPublish(pkg.outputPath, npmDistTag, this.config.publishRegistry);

dev-infra/release/publish/actions/cut-lts-patch.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,14 @@ export class CutLongTermSupportPatchAction extends ReleaseAction {
6565
{
6666
name: 'activeLtsBranch',
6767
type: 'list',
68-
message: 'Please select a version for which you want to cut a LTS patch',
68+
message: 'Please select a version for which you want to cut an LTS patch',
6969
choices: activeBranchChoices,
7070
},
7171
{
7272
name: 'inactiveLtsBranch',
7373
type: 'list',
7474
when: o => o.activeLtsBranch === null,
75-
message: 'Please select an inactive LTS version for which you want to cut a LTS patch',
75+
message: 'Please select an inactive LTS version for which you want to cut an LTS patch',
7676
choices: inactive.map(branch => this._getChoiceForLtsBranch(branch)),
7777
}
7878
]);

dev-infra/release/publish/external-commands.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as Ora from 'ora';
9+
import * as ora from 'ora';
1010
import * as semver from 'semver';
1111

1212
import {spawnWithDebugOutput} from '../../utils/child-process';
@@ -19,11 +19,11 @@ import {FatalReleaseActionError} from './actions-error';
1919
* ###############################################################
2020
*
2121
* This file contains helpers for invoking external `ng-dev` commands. A subset of actions,
22-
* like building release output or setting a NPM dist tag for release packages, cannot be
22+
* like building release output or setting NPM dist tag for release packages, cannot be
2323
* performed directly as part of the release tool and need to be delegated to external `ng-dev`
2424
* commands that exist across arbitrary version branches.
2525
*
26-
* In an concrete example: Consider a new patch version is released and that a new release
26+
* In a concrete example: Consider a new patch version is released and that a new release
2727
* package has been added to the `next` branch. The patch branch will not contain the new
2828
* release package, so we could not build the release output for it. To work around this, we
2929
* call the ng-dev build command for the patch version branch and expect it to return a list
@@ -44,7 +44,7 @@ export async function invokeSetNpmDistCommand(npmDistTag: string, version: semve
4444
info(green(` ✓ Set "${npmDistTag}" NPM dist tag for all packages to v${version}.`));
4545
} catch (e) {
4646
error(e);
47-
error(red(` ✘ An error occurred while setting the NPM dist tag for ${npmDistTag}.`));
47+
error(red(' ✘ An error occurred while setting the NPM dist tag for "${npmDistTag}".'));
4848
throw new FatalReleaseActionError();
4949
}
5050
}
@@ -54,21 +54,21 @@ export async function invokeSetNpmDistCommand(npmDistTag: string, version: semve
5454
* packages for the currently checked out branch.
5555
*/
5656
export async function invokeReleaseBuildCommand(): Promise<BuiltPackage[]> {
57-
const spinner = Ora().start('Building release output.');
57+
const spinner = ora().start('Building release output.');
5858
try {
5959
// Since we expect JSON to be printed from the `ng-dev release build` command,
6060
// we spawn the process in silent mode. We have set up an Ora progress spinner.
6161
const {stdout} = await spawnWithDebugOutput(
6262
'yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], {mode: 'silent'});
6363
spinner.stop();
64-
info(green(` ✓ Built release output for all packages.`));
64+
info(green(' ✓ Built release output for all packages.'));
6565
// The `ng-dev release build` command prints a JSON array to stdout
6666
// that represents the built release packages and their output paths.
6767
return JSON.parse(stdout.trim());
6868
} catch (e) {
6969
spinner.stop();
7070
error(e);
71-
error(red(` ✘ An error occurred while building the release packages.`));
71+
error(red(' ✘ An error occurred while building the release packages.'));
7272
throw new FatalReleaseActionError();
7373
}
7474
}
@@ -83,10 +83,10 @@ export async function invokeYarnInstallCommand(projectDir: string): Promise<void
8383
// TODO: Consider using an Ora spinner instead to ensure minimal console output.
8484
await spawnWithDebugOutput(
8585
'yarn', ['install', '--frozen-lockfile', '--non-interactive'], {cwd: projectDir});
86-
info(green(` ✓ Installed project dependencies.`));
86+
info(green(' ✓ Installed project dependencies.'));
8787
} catch (e) {
8888
error(e);
89-
error(red(` ✘ An error occurred while installing dependencies.`));
89+
error(red(' ✘ An error occurred while installing dependencies.'));
9090
throw new FatalReleaseActionError();
9191
}
9292
}

dev-infra/release/publish/index.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ export class ReleaseTool {
6666
// Only print the error message and stack if the error is not a known fatal release
6767
// action error (for which we print the error gracefully to the console with colors).
6868
if (!(e instanceof FatalReleaseActionError) && e instanceof Error) {
69-
console.error(e.message);
70-
console.error(e.stack);
69+
console.error(e);
7170
}
7271
return CompletionState.FATAL_ERROR;
7372
} finally {
@@ -90,7 +89,7 @@ export class ReleaseTool {
9089
}
9190
}
9291

93-
info(`Please select the type of release you want to perform.`);
92+
info('Please select the type of release you want to perform.');
9493

9594
const {releaseAction} = await prompt<{releaseAction: ReleaseAction}>({
9695
name: 'releaseAction',
@@ -108,9 +107,7 @@ export class ReleaseTool {
108107
*/
109108
private async _verifyNoUncommittedChanges(): Promise<boolean> {
110109
if (this._git.hasUncommittedChanges()) {
111-
error(
112-
red(` ✘ There are changes which are not committed and should be ` +
113-
`discarded.`));
110+
error(red(' ✘ There are changes which are not committed and should be discarded.'));
114111
return false;
115112
}
116113
return true;
@@ -126,7 +123,7 @@ export class ReleaseTool {
126123
await this._git.github.repos.getBranch({...this._git.remoteParams, branch: nextBranchName});
127124

128125
if (headSha !== data.commit.sha) {
129-
error(red(` ✘ Running release tool from an outdated local branch.`));
126+
error(red(' ✘ Running release tool from an outdated local branch.'));
130127
error(red(` Please make sure you are running from the "${nextBranchName}" branch.`));
131128
return false;
132129
}

dev-infra/release/publish/test/cut-lts-patch.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {CutLongTermSupportPatchAction} from '../actions/cut-lts-patch';
1313

1414
import {expectStagingAndPublishWithCherryPick, fakeNpmPackageQueryRequest, getTestingMocksForReleaseAction, parse, setupReleaseActionForTesting, testTmpDir} from './test-utils';
1515

16-
describe('cut a LTS patch action', () => {
16+
describe('cut an LTS patch action', () => {
1717
it('should be active', async () => {
1818
expect(await CutLongTermSupportPatchAction.isActive({
1919
releaseCandidate: null,

dev-infra/release/set-dist-tag/cli.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as Ora from 'ora';
9+
import * as ora from 'ora';
1010
import * as semver from 'semver';
1111
import {Arguments, Argv, CommandModule} from 'yargs';
1212

@@ -15,7 +15,7 @@ import {getReleaseConfig} from '../config/index';
1515
import {setNpmTagForPackage} from '../versioning/npm-publish';
1616

1717

18-
/** Command line options for setting a NPM dist tag. */
18+
/** Command line options for setting an NPM dist tag. */
1919
export interface ReleaseSetDistTagOptions {
2020
tagName: string;
2121
targetVersion: string;
@@ -42,11 +42,11 @@ async function handler(args: Arguments<ReleaseSetDistTagOptions>) {
4242
const version = semver.parse(rawVersion);
4343

4444
if (version === null) {
45-
error(red(`Invalid version specified. Unable to set NPM dist tag.`));
45+
error(red(`Invalid version specified (${rawVersion}). Unable to set NPM dist tag.`));
4646
process.exit(1);
4747
}
4848

49-
const spinner = Ora().start();
49+
const spinner = ora().start();
5050
debug(`Setting "${tagName}" NPM dist tag for release packages to v${version}.`);
5151

5252
for (const pkgName of npmPackages) {
@@ -69,7 +69,7 @@ async function handler(args: Arguments<ReleaseSetDistTagOptions>) {
6969
info(green(` ${bold(tagName)} will now point to ${bold(`v${version}`)}.`));
7070
}
7171

72-
/** CLI command module for setting a NPM dist tag. */
72+
/** CLI command module for setting an NPM dist tag. */
7373
export const ReleaseSetDistTagCommand: CommandModule<{}, ReleaseSetDistTagOptions> = {
7474
builder,
7575
handler,

dev-infra/release/set-dist-tag/set-dist-tag.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('ng-dev release set-dist-tag', () => {
6666
await invokeCommand('latest', '10.0');
6767

6868
expect(console.error)
69-
.toHaveBeenCalledWith('Invalid version specified. Unable to set NPM dist tag.');
69+
.toHaveBeenCalledWith('Invalid version specified (10.0). Unable to set NPM dist tag.');
7070
expect(process.exit).toHaveBeenCalledWith(1);
7171
expect(process.exit).toHaveBeenCalledTimes(1);
7272
});
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
## Versioning for the Angular organization
22

33
The folder contains common tooling needed for implementing the versioning as proposed
4-
by [this design document](https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU/edit#heading=h.s3qlps8f4zq7).
4+
by [this design document](https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU).
55
Primary tooling is the determination of _active_ release trains.

dev-infra/release/versioning/active-release-trains.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export interface ActiveReleaseTrains {
1717
releaseCandidate: ReleaseTrain|null;
1818
/** Release-train currently in the "latest" phase. */
1919
latest: ReleaseTrain;
20-
/** Release-train in the `next` phase */
20+
/** Release-train in the `next` phase. */
2121
next: ReleaseTrain;
2222
}
2323

0 commit comments

Comments
 (0)