[rrm] Rewrite stable-prs command
Now with - better UX (can skip, handle each failed cherr-pick individually) - easier to read code - better error handling / cancellation
Paul O’Shannessy committed
Sep 19, 2016 at 15:29 UTC
b763dc8778105f463ee38ba21a8a9c930ed33010
2 files changed
+225
-133
scripts/release-manager/commands/stable-prs.js
+224
-133
@@ -1,176 +1,267 @@
1
'use strict';
2
3
const chalk = require('chalk');
4
+const promisify = require('es6-promisify');
5
+const pify = require('pify');
6
7
const git = require('./utils/git');
8
7
-// currently 15-next
8
-// IDEA: maybe just always use this milestone across major releases too?
9
-const MILESTONE_NUMBER = 25;
9
+const SEMVER_LABELS = [
10
+ 'semver-major',
11
+ 'semver-minor',
12
+ 'semver-patch',
13
+ 'semver-exempt',
14
+];
15
11
-// 15.3.0
12
-const TARGET_MILESTONE_NUMBER = 31;
13
-
14
-const LABELS = {
15
- // Until there is a UI for it, uncomment these lines to run a patch release instead
16
- // 'semver-patch': true,
17
- // 'semver-exempt': true,
18
-};
19
-
20
-// FOR CODE
21
-// get all closed issues with milestone
22
-// ensure all have pull_request
23
-// FAIL: log issues that aren't prs
24
-// fetch each pr, (issues.pull_request.url)
25
-// sort each by merged_at
26
-// git cherry-pick -x sha || git cherry-pick -x -m1 sha
27
-// (or use API to look up number of parents, 2 = use -m1)
28
-// track progress. on fail, pause and force user to handle manually, continue? prompt
29
-// git push
30
-// update labels on each PR
31
-// ALT: dump link to https://github.com/facebook/react/issues?q=label%3A%22Documentation%3A+needs+merge+to+stable%22+is%3Aclosed
32
-// and say manual step to remove label
16
17
18
module.exports = function(vorpal, app) {
19
vorpal
20
.command('stable-prs')
21
.description('Get list of stable pull requests that need to be merged to the stable branch')
39
- .action(function(args, actionCB) {
40
- // TODO: stop assuming this all fits into a single
41
- const query = {
42
- milestone: MILESTONE_NUMBER,
43
- per_page: 100, // MAX
44
- state: 'closed',
45
- };
46
-
47
- app.ghissues.listIssues(query, (err, body) => {
48
- if (err) {
49
- this.log('ERROR', err);
50
- return actionCB();
51
- }
52
- app.writeTo('stable-issues.json', body);
22
+ .action(function(args) {
23
+ // This makes the chaining easier but obfuscates the actual API, which is
24
+ // unfortunate. The standalone API will return the right data but
25
+ // promisified will get the response object and then we need to pull data
26
+ // off of that.
27
+ let listMilestones = pify(app.ghissues.listMilestones.bind(app.ghissues));
28
+ let listIssues = pify(app.ghissues.listIssues.bind(app.ghissues));
29
+ let editIssue = pify(app.ghissues.editIssue.bind(app.ghissues));
30
+ let getPullRequest = pify(app.ghrepo.getPullRequest.bind(app.ghrepo));
31
32
+ let targetMilestone = null;
33
55
- // This API *could* return issues that aren't pull requests, so filter out
56
- // issues that don't have pull_request set. Also filter out issues that
57
- // aren't the right level of semver (eg if running a patch release)
58
- const filteringLabels = Object.keys(LABELS).length > 0;
59
- const pulls = body.filter((issue) => {
60
- if (!issue.pull_request) {
61
- return false;
62
- }
34
+ return new Promise((resolveAction, rejectAction) => {
35
+ listMilestones(null).then((milestones) => {
36
+ app.writeTo('milestones.json', milestones);
37
64
- if (!filteringLabels) {
65
- return true;
66
- }
38
+ // Turn the milestones into choices for Inquirer
39
+ let milestoneChoices = milestones.map((milestone) => {
40
+ return {
41
+ value: milestone.number,
42
+ name: milestone.title,
43
+ };
44
+ });
45
68
- return issue.labels.some((label) => LABELS[label.name]);
69
- });
46
+ // We need label choices too
47
+ let labelChoices = SEMVER_LABELS.map((label) => {
48
+ return {
49
+ value: label,
50
+ name: label.split('-')[1], // "major" instead of "semver-major"
51
+ };
52
+ });
53
71
- // We don't enough data about the pull request (merge sha or merge time) so we
72
- // need to fetch more. We'll use promises so we don't have to count completions.
73
- const pullPromises = pulls.map((pr) => {
74
- return new Promise((resolve, reject) => {
75
- app.ghrepo.getPullRequest(pr.number, (err, body) => {
76
- if (err) {
77
- reject(err);
78
- }
54
+ // Ask about source milestone
55
+ // Ask about dest milestone
56
+ // TODO: allow creation of milestone here.
57
+ // Ask about which labels to pull from
58
80
- app.writeTo(`pr-${pr.number}.json`, body);
81
- // We want to track the original issue as well since it has the
82
- // milestone & label information.
83
- const richPull = body;
84
- richPull.__originalIssue = pr;
85
- resolve(richPull);
59
+ return this.prompt([
60
+ {
61
+ name: 'srcMilestone',
62
+ type: 'list',
63
+ message: 'Which milestone should we pull PRs from?',
64
+ choices: milestoneChoices,
65
+ },
66
+ {
67
+ name: 'destMilestone',
68
+ type: 'list',
69
+ message: 'Which milestone should we assign PRs to upon completion?',
70
+ choices: milestoneChoices,
71
+ },
72
+ {
73
+ name: 'labels',
74
+ type: 'checkbox',
75
+ message: 'Which PRs should we select (use spacebar to check all that apply)',
76
+ choices: labelChoices,
77
+ },
78
+ ]).then((answers) => {
79
+ // this.log(JSON.stringify(answers, null, 2));
80
+ targetMilestone = answers.destMilestone;
81
+ let labels = {};
82
+ answers.labels.forEach((label) => {
83
+ labels[label] = true;
84
});
85
+ return {
86
+ labels: labels,
87
+ query: {
88
+ milestone: answers.srcMilestone,
89
+ per_page: 100,
90
+ state: 'closed',
91
+ },
92
+ };
93
});
88
- });
89
- Promise.all(pullPromises).then((richPulls) => {
90
- // Remove any pull requests that were closed but not merged. We don't
91
- // care about those. They shouldn't have the milestone set anyway.
92
- // Log for that so they can be manually modified.
93
- richPulls = richPulls.filter((pr) => {
94
- if (!pr.merged_at) {
95
- this.log(`${chalk.yellow.bold('WARNING')} ${pr.html_url} was not merged, should have the milestone unset.`);
96
- return false;
97
- }
98
- return true;
99
- });
100
- richPulls.forEach((pr) => {
101
- // Convert merged_at to real Date for sorting
102
- pr.merged_at_date = new Date(pr.merged_at);
103
- });
104
-
105
- richPulls = richPulls.sort((a, b) => a.merged_at_date - b.merged_at_date);
94
+ })
95
+ // Request issues, filter to applicable PRs
96
+ .then(({labels, query}) => {
97
+ return listIssues(query).then((issues) => {
98
+ app.writeTo('stable-issues.json', issues);
99
107
- this.log(`Found ${chalk.bold(richPulls.length)} pull requests:`);
100
+ // This API *could* return issues that aren't pull requests, so filter out
101
+ // issues that don't have pull_request set. Also filter out issues that
102
+ // aren't the right level of semver (eg if running a patch release)
103
+ let filteringLabels = Object.keys(labels).length > 0;
104
+ const pulls = issues.filter((issue) => {
105
+ if (!issue.pull_request) {
106
+ return false;
107
+ }
108
109
- promptForPRs.call(this, app, richPulls, 0).then(() => {
109
+ if (!filteringLabels) {
110
+ return true;
111
+ }
112
111
- // Update the milestone
112
- if (!TARGET_MILESTONE_NUMBER) {
113
- return actionCB();
114
- }
113
+ return issue.labels.some((label) => labels[label.name]);
114
+ });
115
+ app.writeTo('stable-prs.json', pulls);
116
+ return pulls;
117
+ })
118
+ // We need to convert the issues to PRs. We don't actually have enough
119
+ // info for the pull request data, so we need to get more. Then we'll
120
+ // do some filtering and sorting to make sure we apply merged PRs in
121
+ // the order they were originally committed to avoid conflicts as much
122
+ // as possible.
123
+ .then((pulls) => {
124
+ return Promise.all(pulls.map((pr) => {
125
+ return getPullRequest(pr.number)
126
+ .then((richPR) => {
127
+ app.writeTo(`pr-${pr.number}.json`, richPR);
128
+ richPR.__originalIssue = pr;
129
+ return richPR;
130
+ });
131
+ }))
132
+ .then((richPRs) => {
133
+ return richPRs.filter((pr) => {
134
+ if (!pr.merged_at) {
135
+ this.log(`${chalk.yellow.bold('WARNING')} ${pr.html_url} was not merged, should have the milestone unset.`);
136
+ return false;
137
+ }
138
+ return true;
139
+ }).map((pr) => {
140
+ pr.merged_at_date = new Date(pr.merged_at);
141
+ return pr;
142
+ }).sort((a, b) => a.merged_at_date - b.merged_at_date);
143
+ });
144
+ });
145
+ })
146
+ // Quick prompt to double check that we should proceed.
147
+ .then((pulls) => {
148
+ this.log(`Found ${chalk.bold(pulls.length)} pull requests:`);
149
+ pulls.forEach((pr) => {
150
+ this.log(`${pr.html_url}: ${chalk.bold(pr.title)}`);
151
+ });
152
116
- const milestonePromises = richPulls.map((pr) => {
117
- return app.ghissues.editIssue(pr.number, {
118
- milestone: TARGET_MILESTONE_NUMBER,
153
+ return this.prompt({
154
+ name: 'merge',
155
+ type: 'confirm',
156
+ message: `Merge these ${pulls.length} pull requests?`,
157
+ }).then((answers) => {
158
+ return answers.merge ? pulls : rejectAction('cancelled');
159
+ });
160
+ })
161
+ // Ok, now we finally have rich pull request data. We can start cherry picking…
162
+ .then((pulls) => {
163
+ // We're going to do some error handling here so we don't get into a
164
+ // terrible state.
165
+ this.log(`Found ${chalk.bold(pulls.length)} pull requests:`);
166
+ return new Promise((resolve, reject) => {
167
+ cherryPickPRs.call(this, app, pulls)
168
+ .then((results) => {
169
+ resolve(results);
170
+ })
171
+ .catch((err) => {
172
+ this.log(`${chalk.red.bold('ERROR')} Something went wrong and your repo is probably in a bad state. Sorry.`);
173
+ resolve({
174
+ successful: [],
175
+ skipped: [],
176
+ didAbort: true,
177
+ });
178
});
120
- });
121
- Promise.all(milestonePromises).then(actionCB);
179
});
123
- });
180
+ })
181
+ .then(({successful, skipped, didAbort}) => {
182
+ console.log(successful.length, skipped.length, didAbort);
183
+ })
184
+ // Update the milestone on successful PRs
185
+ // // TODO: maybe handle didAbort and git reset --hard to a rev we read when we start the process?
186
+ .then(({successful, aborted, didAbort}) => {
187
+ if (didAbort) {
188
+ return undefined;
189
+ }
190
191
+ return Promise.all(successful.map((pr) => {
192
+ return editIssue(pr.number, {milestone: targetMilestone});
193
+ }));
194
+ })
195
+ // yay, we're done
196
+ .then(() => {
197
+ resolveAction();
198
+ })
199
+ .catch((err) => {
200
+ this.log('ERROR', err);
201
+ rejectAction();
202
+ });
203
});
126
-
204
});
205
};
206
130
-
131
-// TODO: pull this out to some shared place. We can reuse this for docs.
132
-function promptForPRs(app, prs, start) {
207
+function cherryPickPRs(app, prs) {
208
+ let successful = [];
209
+ let skipped = [];
210
return new Promise((resolve, reject) => {
134
- const choices = prs.map((pr, idx) => {
135
- return {
136
- value: idx,
137
- name: `${pr.html_url}: ${chalk.bold(pr.title)}`,
138
- };
139
- });
140
- // TODO: add a seperator so we know when we loop around. doesn't look like
141
- // the "native" inquirer one is exposed so maybe build our own.
142
-
143
- this.prompt({
144
- name: 'start',
145
- type: 'list',
146
- message: 'Start from?',
147
- default: start,
148
- choices: choices,
149
- }).then((res) => {
150
-
151
- let failed = false;
152
-
153
- for (let i = +res.start; i < prs.length; i++) {
154
- let pr = prs[i];
155
- this.log(chalk.grey.italic(`Cherry-picking ${pr.number}`));
211
+ // Build array of thenables
212
+ let promises = prs.map((pr) => {
213
+ return () => new Promise((res, rej) => {
214
+ this.log(chalk.yellow(`Cherry-picking #${pr.number} (${pr.title})...`));
215
+ let failed = false;
216
try {
217
git.cherryPickMerge(app, pr.merge_commit_sha);
218
} catch (e) {
159
-
160
- // TODO: add ability to mark a PR as skipped
219
failed = true;
162
- this.log(`${chalk.bold.red('FAILED!')} Please fix manually and continue when ready.`);
163
- promptForPRs.call(this, app, prs, i + 1).then(resolve);
164
- break;
220
}
166
- }
221
168
- // Make sure we resolve in case there were no issues
169
- if (!failed) {
170
- resolve();
171
- }
222
+ if (!failed) {
223
+ this.log(chalk.green`Success`);
224
+ successful.push(pr);
225
+ return res();
226
+ }
227
+
228
+ return this.prompt({
229
+ name: 'handle',
230
+ type: 'list',
231
+ message: `${chalk.red`Failed!`} ${chalk.yellow('This must be resolved manually!')}`,
232
+ choices: [
233
+ {value: 'ok', name: 'Continue, mark successful'},
234
+ {value: 'skip', name: 'Continue, mark skipped'},
235
+ {value: 'abort', name: 'Abort process. Will require manual resetting of git state.'},
236
+ ],
237
+ }).then((answers) => {
238
+ switch (answers.handle) {
239
+ case 'ok':
240
+ successful.push(pr);
241
+ break;
242
+ case 'skip':
243
+ skipped.push(pr);
244
+ break;
245
+ case 'abort':
246
+ return rej(pr.number);
247
+ }
248
+ res(pr.number);
249
+ });
250
+ });
251
});
173
- });
252
253
+ // Since promises run on creation and we don't actually want that, we create
254
+ // an array of functions that return promises. We'll chain them here, not
255
+ // actually creating the next promise until we're ready.
256
+ var p = promises[0]();
257
+ for (let i = 1; i < promises.length; i++) {
258
+ p = p.then(() => promises[i]());
259
+ }
260
+ p.then(() => {
261
+ resolve({successful, skipped, didAbort: false});
262
+ }).catch((e) => {
263
+ resolve({successful, skipped, didAbort: true});
264
+ });
265
266
+ });
267
}
scripts/release-manager/package.json
+1
@@ -10,6 +10,7 @@
10
"github-api": "^2.2.0",
11
"glob": "^7.0.5",
12
"opn": "^4.0.2",
13
+ "pify": "^2.3.0",
14
"semver": "^5.3.0",
15
"untildify": "^3.0.2",
16
"vorpal": "^1.10.10"