Skip to content

Commit b27d257

Browse files
codebyterecodebytere
committed
refactor: address review comments
1 parent 5ba3eb3 commit b27d257

File tree

3 files changed

+58
-72
lines changed

3 files changed

+58
-72
lines changed

‎components/git/release.js‎

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,46 @@
33
const semver = require('semver');
44
const yargs = require('yargs');
55

6-
const auth = require('../../lib/auth');
76
const CLI = require('../../lib/cli');
8-
const Release = require('../../lib/release');
9-
const Request = require('../../lib/request');
10-
const TeamInfo = require('../../lib/team_info');
7+
const ReleasePreparation = require('../../lib/prepare_release');
118
const { runPromise } = require('../../lib/run');
129

1310
const PREPARE = 'prepare';
1411
const PROMOTE = 'promote';
1512

16-
const RELEASERS = 'releasers';
17-
1813
const releaseOptions = {
1914
prepare: {
2015
describe: 'Prepare a new release with the given version number',
2116
type: 'boolean'
2217
},
18+
promote: {
19+
describe: 'Promote new release with the given version number',
20+
type: 'boolean'
21+
},
2322
security: {
24-
describe: 'Prepare a new security release',
23+
describe: 'Demarcate the new security release as a security release',
2524
type: 'boolean'
2625
}
2726
};
2827

2928
function builder(yargs) {
3029
return yargs
3130
.options(releaseOptions).positional('newVersion', {
32-
describe: 'Version number of the release to be created'
31+
describe: 'Version number of the release to be prepared or promoted'
3332
})
34-
.example('git node release 1.2.3',
33+
.example('git node release --prepare 1.2.3',
3534
'Prepare a new release of Node.js tagged v1.2.3');
3635
}
3736

3837
function handler(argv) {
3938
if (argv.newVersion) {
4039
const newVersion = semver.coerce(argv.newVersion);
4140
if (semver.valid(newVersion)) {
42-
return release(PREPARE, argv);
41+
if (argv.prepare) {
42+
return release(PREPARE, argv);
43+
} else if (argv.promote) {
44+
return release(PROMOTE, argv);
45+
}
4346
}
4447
}
4548

@@ -51,11 +54,9 @@ function handler(argv) {
5154
function release(state, argv) {
5255
const logStream = process.stdout.isTTY ? process.stdout : process.stderr;
5356
const cli = new CLI(logStream);
54-
55-
const req = new Request();
5657
const dir = process.cwd();
5758

58-
return runPromise(main(state, argv, cli, req, dir)).catch((err) => {
59+
return runPromise(main(state, argv, cli, dir)).catch((err) => {
5960
if (cli.spinner.enabled) {
6061
cli.spinner.fail();
6162
}
@@ -71,34 +72,23 @@ module.exports = {
7172
handler
7273
};
7374

74-
async function main(state, argv, cli, req, dir) {
75-
const release = new Release(state, argv, cli, req, dir);
76-
77-
cli.startSpinner('Verifying Releaser status');
78-
const credentials = await auth({ github: true });
79-
const request = new Request(credentials);
80-
const info = new TeamInfo(cli, request, 'nodejs', RELEASERS);
81-
const releasers = await info.getMembers();
82-
if (!releasers.some(r => r.login === release.username)) {
83-
cli.stopSpinner(`${release.username} is not a Releaser; aborting release`);
84-
return;
85-
}
86-
cli.stopSpinner('Verified Releaser status');
87-
75+
async function main(state, argv, cli, dir) {
8876
if (state === PREPARE) {
89-
if (release.warnForWrongBranch()) return;
77+
const prep = new ReleasePreparation(argv, cli, dir);
78+
79+
if (prep.warnForWrongBranch()) return;
9080

9181
// Check the branch diff to determine if the releaser
9282
// wants to backport any more commits before proceeding.
9383
cli.startSpinner('Fetching branch-diff');
94-
const raw = release.getBranchDiff();
84+
const raw = prep.getBranchDiff();
9585
const diff = raw.split('*');
9686
cli.stopSpinner('Got branch diff');
9787

9888
const staging = `v${semver.major(argv.newVersion)}.x-staging`;
9989
const proceed = await cli.prompt(
100-
`There are ${diff.length} commits that may be backported ` +
101-
`to ${staging} - do you still want to proceed?`,
90+
`There are ${diff.length - 1} commits that may be ` +
91+
`backported to ${staging} - do you still want to proceed?`,
10292
false);
10393

10494
if (!proceed) {
@@ -108,8 +98,8 @@ async function main(state, argv, cli, req, dir) {
10898
return;
10999
}
110100

111-
return release.prepare();
101+
return prep.prepare();
112102
} else if (state === PROMOTE) {
113-
return release.promote();
103+
// TODO(codebytere): implement release promotion.
114104
}
115105
}

lib/release.js renamed to lib/prepare_release.js

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ const { getMergedConfig } = require('./config');
99
const { runAsync, runSync } = require('./run');
1010
const { writeJson, readJson } = require('./file');
1111

12-
class Release {
13-
constructor(state, argv, cli, req, dir) {
12+
const isWindows = process.platform === 'win32';
13+
14+
class ReleasePreparation {
15+
constructor(argv, cli, req, dir) {
1416
this.cli = cli;
1517
this.dir = dir;
1618
this.newVersion = argv.newVersion;
@@ -49,10 +51,9 @@ class Release {
4951
await this.createProposalBranch();
5052

5153
// Update version and release info in src/node_version.h.
52-
const shouldUpdateNodeVersion = await cli.prompt(
53-
`Update 'src/node_version.h' for ${newVersion}?`, true);
54-
if (!shouldUpdateNodeVersion) return this.abort();
54+
cli.startSpinner(`Updating 'src/node_version.h' for ${newVersion}`);
5555
await this.updateNodeVersion();
56+
cli.stopSpinner(`Updating 'src/node_version.h' for ${newVersion}`);
5657

5758
// Check whether to update NODE_MODULE_VERSION (default false).
5859
const shouldUpdateNodeModuleVersion = await cli.prompt(
@@ -71,19 +72,14 @@ class Release {
7172
}
7273

7374
// Update any REPLACEME tags in the docs.
74-
const shouldUpdateREPLACEMEs = await cli.prompt(
75-
'Update REPLACEME items in docs?', true);
76-
if (!shouldUpdateREPLACEMEs) return this.abort();
75+
cli.startSpinner('Updating REPLACEME items in docs');
7776
await this.updateREPLACEMEs();
77+
cli.stopSpinner('Updated REPLACEME items in docs');
7878

79+
// Fetch date to use in release commit & changelogs.
7980
this.date = await cli.promptInput(
8081
'Enter release date in YYYY-MM-DD format:');
8182

82-
// Update Changelogs
83-
const shouldUpdateChangelogs = await cli.prompt(
84-
'Update changelogs?', true);
85-
if (!shouldUpdateChangelogs) return this.abort();
86-
8783
cli.startSpinner('Updating CHANGELOG.md');
8884
await this.updateMainChangelog();
8985
cli.stopSpinner('Updated CHANGELOG.md');
@@ -101,8 +97,8 @@ class Release {
10197

10298
// Proceed with release only after the releaser has amended
10399
// it to their liking.
104-
const created = await this.createReleaseCommit();
105-
if (!created) {
100+
const createDefaultCommit = await this.createReleaseCommit();
101+
if (!createDefaultCommit) {
106102
const lastCommitSha = runSync('git', ['rev-parse', '--short', 'HEAD']);
107103
cli.warn(`Please manually edit commit ${lastCommitSha} by running ` +
108104
'`git commit --amend` before proceeding.');
@@ -130,10 +126,6 @@ class Release {
130126
);
131127
}
132128

133-
async promote() {
134-
// TODO(codebytere): implement.
135-
}
136-
137129
get owner() {
138130
return this.config.owner || 'nodejs';
139131
}
@@ -167,9 +159,14 @@ class Release {
167159
}
168160

169161
getChangelog() {
170-
return runSync('npx', [
171-
'changelog-maker',
162+
const changelogMaker = path.join(
163+
__dirname,
164+
'../node_modules/.bin/changelog-maker' + (isWindows ? '.cmd' : '')
165+
);
166+
167+
return runSync(changelogMaker, [
172168
'--group',
169+
'--filter-release',
173170
'--start-ref',
174171
this.getLastRef()
175172
]).trim();
@@ -245,15 +242,8 @@ class Release {
245242
const data = await fs.readFile(majorChangelogPath, 'utf8');
246243
const arr = data.split('\n');
247244

248-
const allCommits = runSync('npx', [
249-
'changelog-maker',
250-
'--group',
251-
'--filter-release',
252-
'--start-ref',
253-
lastRef
254-
]);
255-
256-
const notableChanges = this.getBranchDiff(true);
245+
const allCommits = this.getChangelog();
246+
const notableChanges = this.getBranchDiff({ onlyNotableChanges: true });
257247
const releaseHeader = `## ${date}, Version ${newVersion}` +
258248
` ${releaseInfo}, @${username}\n`;
259249

@@ -334,7 +324,7 @@ class Release {
334324
messageBody.push('This is a security release.\n\n');
335325
}
336326

337-
const notableChanges = this.getBranchDiff(true);
327+
const notableChanges = this.getBranchDiff({ onlyNotableChanges: true });
338328
messageBody.push('Notable changes:\n\n');
339329
messageBody.push(notableChanges);
340330

@@ -349,11 +339,12 @@ class Release {
349339
]);
350340

351341
cli.log(`${messageTitle}\n\n${messageBody.join('')}`);
352-
const useMessage = await cli.prompt('Continue with this commit message?', true);
342+
const useMessage = await cli.prompt(
343+
'Continue with this commit message?', true);
353344
return useMessage;
354345
}
355346

356-
getBranchDiff(onlyNotableChanges = false) {
347+
getBranchDiff(opts) {
357348
const {
358349
versionComponents,
359350
stagingBranch,
@@ -363,7 +354,7 @@ class Release {
363354
} = this;
364355

365356
let branchDiffOptions;
366-
if (onlyNotableChanges) {
357+
if (opts.onlyNotableChanges) {
367358
const proposalBranch = `v${newVersion}-proposal`;
368359
const releaseBranch = `v${versionComponents.major}.x`;
369360

@@ -373,7 +364,6 @@ class Release {
373364
];
374365

375366
branchDiffOptions = [
376-
'branch-diff',
377367
`${upstream}/${releaseBranch}`,
378368
proposalBranch,
379369
`--require-label=${notableLabels.join(',')}`,
@@ -388,12 +378,12 @@ class Release {
388378
`backport-blocked-v${versionComponents.major}.x`
389379
];
390380

391-
if (isLTS) {
381+
const isSemverMinor = versionComponents.patch === 0;
382+
if (isLTS && !isSemverMinor) {
392383
excludeLabels.push('semver-minor');
393384
}
394385

395386
branchDiffOptions = [
396-
'branch-diff',
397387
stagingBranch,
398388
// TODO(codebytere): use Current branch instead of master for LTS
399389
'master',
@@ -403,7 +393,12 @@ class Release {
403393
];
404394
}
405395

406-
return runSync('npx', branchDiffOptions);
396+
const branchDiff = path.join(
397+
__dirname,
398+
'../node_modules/.bin/branch-diff' + (isWindows ? '.cmd' : '')
399+
);
400+
401+
return runSync(branchDiff, branchDiffOptions);
407402
}
408403

409404
warnForWrongBranch() {
@@ -434,4 +429,4 @@ class Release {
434429
}
435430
}
436431

437-
module.exports = Release;
432+
module.exports = ReleasePreparation;

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
],
3232
"license": "MIT",
3333
"dependencies": {
34+
"branch-diff": "^1.8.1",
3435
"chalk": "^3.0.0",
3536
"cheerio": "^1.0.0-rc.3",
3637
"clipboardy": "^2.1.0",

0 commit comments

Comments
 (0)