Skip to content

Commit 60018cc

Browse files
joyeecheungjoyeecheung
committed
backport: improve message
1 parent d42af3b commit 60018cc

5 files changed

Lines changed: 76 additions & 33 deletions

File tree

‎components/git/backport.js‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,19 @@ const { runPromise } = require('../../lib/run');
77
const BackportSession = require('../../lib/backport_session');
88

99
const epilogue = `====================== Example =======================
10-
Backporting https://github.com/nodejs/node/pull/12344 to v10.x
10+
Demo: https://asciinema.org/a/221244
11+
Backporting https://github.com/nodejs/node/pull/24816 to v11.x
1112
1213
# Sync master with upstream for the commits, if they are not yet there
1314
$ git checkout master
15+
$ ncu-config set branch master
1416
$ git node sync
1517
16-
# Backport existing commits from master to v10.x-staging
17-
$ git checkout v10.x-staging
18+
# Backport existing commits from master to v11.x-staging
19+
$ git checkout v11.x-staging
20+
$ ncu-config set branch v11.x-staging
1821
$ git node sync
19-
$ git node backport 12344 --to 10
22+
$ git node backport 24816 --to 11
2023
=====================================================
2124
`;
2225

@@ -40,6 +43,7 @@ function builder(yargs) {
4043
async function main(config) {
4144
const logStream = process.stdout.isTTY ? process.stdout : process.stderr;
4245
const cli = new CLI(logStream);
46+
cli.setFigureIndent(0);
4347
const dir = process.cwd();
4448
const session = new BackportSession(cli, dir, config.prid, config.to);
4549
return session.backport();
@@ -64,7 +68,7 @@ function handler(argv) {
6468

6569
module.exports = {
6670
command: 'backport <identifier>',
67-
describe: 'Backport a PR',
71+
describe: 'Backport a PR to a release staging branch.',
6872
builder,
6973
handler
7074
};

‎components/git/sync.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const SyncSession = require('../../lib/sync_session');
66

77
function builder(yargs) {
88
return yargs
9+
.epilogue('Demo: https://asciinema.org/a/221230')
910
.wrap(90);
1011
}
1112

@@ -23,7 +24,7 @@ function handler(argv) {
2324

2425
module.exports = {
2526
command: 'sync',
26-
describe: 'Sync the branch specified by ncu-config',
27+
describe: 'Sync the branch specified by ncu-config.',
2728
builder,
2829
handler
2930
};

‎lib/backport_session.js‎

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ const OLDEST_ID = new Map([
1515

1616
class BackportSession extends Session {
1717
constructor(cli, dir, prid, target) {
18-
// eslint-disable-next-line no-useless-constructor
1918
super(cli, dir, prid);
2019
this.target = target;
2120
}
@@ -27,9 +26,19 @@ class BackportSession extends Session {
2726
}
2827

2928
getPreviousCommits(rev, file, num) {
30-
return runSync('git',
31-
['log', `-${num}`, '--format=%h', rev, file]
32-
).trim().split('\n');
29+
let logs;
30+
try {
31+
logs = runSync('git',
32+
['log', `-${num}`, '--format=%h', rev, '--', file]
33+
).trim();
34+
} catch (e) {
35+
return null;
36+
}
37+
if (!logs) {
38+
return [];
39+
}
40+
41+
return logs.trim().split('\n');
3342
}
3443

3544
getCommitMessage(rev) {
@@ -43,28 +52,48 @@ class BackportSession extends Session {
4352
}
4453

4554
getPotentialConflicts(rev, targetBranch) {
55+
const { cli } = this;
4656
const files = this.getChangedFiles(rev);
4757
const notBackported = new Map();
4858
const oldest = OLDEST_ID.get(this.target);
4959
for (const file of files) {
60+
cli.startSpinner(`Analyzing ancestors of ${file}`);
61+
// TODO(joyeecheung): if the file does not exit in the current revision,
62+
// warn about it and skip it.
5063
const ancestors = this.getPreviousCommits(`${rev}~1`, file, MAX_HISTORY);
51-
this.cli.log(`Analyzing ancestors of ${file}`);
64+
if (!ancestors) {
65+
cli.stopSpinner(`${file} does not exist in current working tree`,
66+
cli.SPINNER_STATUS.WARN);
67+
continue;
68+
};
69+
if (ancestors.length === 0) {
70+
cli.stopSpinner(`Cannot find ancestor commits of ${file}`,
71+
cli.SPINNER_STATUS.INFO);
72+
continue;
73+
}
5274
for (const ancestor of ancestors) {
5375
const message = this.getCommitMessage(ancestor);
76+
cli.updateSpinner(`Analyzing ${message.split('\n')[0]}...`);
5477
let data = parsePrURL(message);
5578
if (!data) {
56-
const match = message.match('/^PR-URL: #(\d+)/');
79+
const match = message.match('/^PR-URL: #(\\d+)/');
5780
if (!match) {
58-
cli.warn(`Commit message of ${ancestor} is ill-formed, skipping`);
81+
cli.stopSpinner(
82+
`Commit message of ${ancestor} is ill-formed, skipping`,
83+
cli.SPINNER_STATUS.WARN);
84+
cli.startSpinner(`Analyzing ancestors of ${file}`);
5985
continue;
6086
}
6187
data = {
6288
repo: this.repo,
6389
owner: this.owner,
6490
prid: parseInt(match[1])
65-
}
91+
};
6692
}
6793
if (data.prid < oldest) {
94+
cli.updateSpinner(
95+
`Commit ${ancestor} iS too old, skipping`,
96+
cli.SPINNER_STATUS.WARN);
6897
break;
6998
}
7099
const backported = this.getCommitsFromBranch(
@@ -80,36 +109,40 @@ class BackportSession extends Session {
80109
url: getPrURL(data),
81110
commit: ancestor,
82111
title: message.split('\n')[0],
83-
files: new Set([file]),
112+
files: new Set([file])
84113
});
85114
}
86115
}
87116
}
117+
cli.stopSpinner(`Analyzed ${file}`);
88118
}
89119
return notBackported;
90120
}
91121

92122
warnForPotentialConflicts(rev) {
93123
const { cli } = this;
94124
const staging = this.stagingBranch;
125+
126+
cli.log(`Looking for potential conflicts of ${rev}...`);
95127
const notBackported = this.getPotentialConflicts(rev, staging);
96128

97129
if (notBackported.size === 0) {
130+
cli.info(`All ancestor commits of ${rev} have been backported`);
98131
return;
99132
}
100133

101-
cli.warn(`The following ancestor commits of ${rev} is not on ${staging}`);
134+
cli.warn(`The following ancestor commits of ${rev} are not on ${staging}`);
102135
for (const [commit, data] of notBackported) {
103-
cli.log(` - ${commit} ${data.title}, ${data.url}`);
136+
cli.log(` - ${commit} ${data.title}, ${data.url}`);
104137
for (const file of data.files) {
105-
cli.log(` ${file}`);
138+
cli.log(` ${file}`);
106139
}
107140
}
108141
}
109142

110143
async backport() {
111144
const { cli } = this;
112-
145+
// TODO(joyeechuneg): add more warnings
113146
const { prid } = this;
114147
const url = getPrURL(this);
115148
cli.log(`Looking for commits of ${url} on master...`);
@@ -139,15 +172,14 @@ class BackportSession extends Session {
139172
'(this could take a while)');
140173
if (shouldAnalyze) {
141174
for (const commit of commits) {
142-
cli.log(`Looking for potential conflicts of ${commit}...`);
143175
this.warnForPotentialConflicts(commit.sha);
144176
}
145177
}
146178

147179
const cherries = commits.map(i => i.sha).reverse();
148180
const pendingCommands = [
149181
`git cherry-pick ${cherries.join(' ')}`,
150-
`git push -u <your-fork-remote> ${newBranch}`
182+
`git push -u <your-fork-remote> <your-branch-name>`
151183
];
152184
const shouldPick = await cli.prompt(
153185
'Do you want to cherry-pick the commits?');
@@ -171,8 +203,8 @@ class BackportSession extends Session {
171203

172204
getCommitsFromBranch(prid, branch, loose = true) {
173205
let re;
174-
const url = getPrURL(this);
175-
re = `--grep=PR-URL: ${url}$`;
206+
const url = getPrURL({ prid, repo: this.repo, owner: this.owner });
207+
re = `--grep=PR-URL: ${url}`;
176208

177209
let commits = runSync('git', [
178210
'log', re, '--format=%h %s', branch
@@ -189,13 +221,13 @@ class BackportSession extends Session {
189221
return [];
190222
}
191223
}
192-
224+
193225
return commits.split('\n').map((i) => {
194226
const match = i.match(/(\w+) (.+)/);
195227
return {
196228
sha: match[1],
197229
title: match[2]
198-
}
230+
};
199231
});
200232
}
201233
}

‎lib/cli.js‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ const inquirer = require('inquirer');
88
const { IGNORE } = require('./run');
99
const { warning, error, info, success } = require('./figures');
1010

11-
const FIGURE_INDENT = ' ';
12-
const EOL_INDENT = `\n${FIGURE_INDENT}`;
13-
1411
const SPINNER_STATUS = {
1512
SUCCESS: 'success',
1613
FAILED: 'failed',
@@ -28,6 +25,15 @@ class CLI {
2825
this.stream = stream || process.stderr;
2926
this.spinner = ora({ stream: this.stream });
3027
this.SPINNER_STATUS = SPINNER_STATUS;
28+
this.figureIndent = ' ';
29+
}
30+
31+
get eolIndent() {
32+
return `\n${this.figureIndent}`;
33+
}
34+
35+
setFigureIndent(indent) {
36+
this.figureIndent = ' '.repeat(indent);
3137
}
3238

3339
promptForInput(question) {
@@ -111,22 +117,22 @@ class CLI {
111117
}
112118

113119
ok(text, options = {}) {
114-
const prefix = options.newline ? EOL_INDENT : FIGURE_INDENT;
120+
const prefix = options.newline ? this.eolIndent : this.figureIndent;
115121
this.log(`${prefix}${success} ${text}`);
116122
}
117123

118124
warn(text, options = {}) {
119-
const prefix = options.newline ? EOL_INDENT : FIGURE_INDENT;
125+
const prefix = options.newline ? this.eolIndent : this.figureIndent;
120126
this.log(prefix + chalk.bold(`${warning} ${text}`));
121127
}
122128

123129
info(text, options = {}) {
124-
const prefix = options.newline ? EOL_INDENT : FIGURE_INDENT;
130+
const prefix = options.newline ? this.eolIndent : this.figureIndent;
125131
this.log(`${prefix}${info} ${text}`);
126132
}
127133

128134
error(obj, options = {}) {
129-
const prefix = options.newline ? EOL_INDENT : FIGURE_INDENT;
135+
const prefix = options.newline ? this.eolIndent : this.figureIndent;
130136
if (obj instanceof Error) {
131137
this.log(`${prefix}${error} ${obj.message}`);
132138
this.log(`${obj.stack}`);

‎lib/sync_session.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
const Session = require('./session');
44

55
class SyncSession extends Session {
6+
// eslint-disable-next-line no-useless-constructor
67
constructor(cli, dir) {
7-
// eslint-disable-next-line no-useless-constructor
88
super(cli, dir);
99
}
1010

0 commit comments

Comments
 (0)