Skip to content

Commit 980e592

Browse files
ckerrckerr
authored
chore: ensure release notes always come from Clerk (electron#23777)
* chore: ensure release notes always come from Clerk Now with tests! * chore: move sinon devDependency into `spec-main` * refactor: tweak note-spec variable for readability
1 parent c6c022d commit 980e592

25 files changed

Lines changed: 372 additions & 72 deletions

‎script/release/notes/notes.js‎

Lines changed: 48 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ const { ELECTRON_VERSION, SRC_DIR } = require('../../lib/utils');
1818
const MAX_FAIL_COUNT = 3;
1919
const CHECK_INTERVAL = 5000;
2020

21-
const CACHE_DIR = path.resolve(__dirname, '.cache');
2221
const NO_NOTES = 'No notes';
2322
const FOLLOW_REPOS = ['electron/electron', 'electron/node'];
2423

@@ -28,6 +27,8 @@ const fixTypes = new Set(['fix']);
2827
const otherTypes = new Set(['spec', 'build', 'test', 'chore', 'deps', 'refactor', 'tools', 'vendor', 'perf', 'style', 'ci']);
2928
const knownTypes = new Set([...docTypes.keys(), ...featTypes.keys(), ...fixTypes.keys(), ...otherTypes.keys()]);
3029

30+
const getCacheDir = () => process.env.NOTES_CACHE_PATH || path.resolve(__dirname, '.cache');
31+
3132
/**
3233
***
3334
**/
@@ -53,9 +54,7 @@ class Commit {
5354
this.owner = owner; // string
5455
this.repo = repo; // string
5556

56-
this.body = null; // string
5757
this.isBreakingChange = false;
58-
this.issueNumber = null; // number
5958
this.note = null; // string
6059
this.prKeys = new Set(); // GHKey
6160
this.revertHash = null; // string
@@ -129,41 +128,11 @@ const OMIT_FROM_RELEASE_NOTES_KEYS = [
129128
'blank'
130129
];
131130

132-
const getNoteFromBody = body => {
133-
if (!body) {
134-
return null;
135-
}
136-
137-
const NOTE_PREFIX = 'Notes: ';
138-
const NOTE_HEADER = '#### Release Notes';
139-
140-
let note = body
141-
.split(/\r?\n\r?\n/) // split into paragraphs
142-
.map(paragraph => paragraph.trim())
143-
.map(paragraph => paragraph.startsWith(NOTE_HEADER) ? paragraph.slice(NOTE_HEADER.length).trim() : paragraph)
144-
.find(paragraph => paragraph.startsWith(NOTE_PREFIX));
145-
146-
if (note) {
147-
note = note
148-
.slice(NOTE_PREFIX.length)
149-
.replace(/<!--.*-->/, '') // '<!-- change summary here-->'
150-
.replace(/\r?\n/, ' ') // remove newlines
151-
.trim();
152-
}
153-
154-
if (note && OMIT_FROM_RELEASE_NOTES_KEYS.includes(note.toLowerCase())) {
155-
return NO_NOTES;
156-
}
157-
158-
return note;
159-
};
160-
161131
/**
162132
* Looks for our project's conventions in the commit message:
163133
*
164134
* 'semantic: some description' -- sets semanticType, subject
165135
* 'some description (#99999)' -- sets subject, pr
166-
* 'Fixes #3333' -- sets issueNumber
167136
* 'Merge pull request #99999 from ${branchname}' -- sets pr
168137
* 'This reverts commit ${sha}' -- sets revertHash
169138
* line starting with 'BREAKING CHANGE' in body -- sets isBreakingChange
@@ -181,13 +150,6 @@ const parseCommitMessage = (commitMessage, commit) => {
181150
subject = subject.slice(0, pos).trim();
182151
}
183152

184-
if (body) {
185-
commit.body = body;
186-
187-
const note = getNoteFromBody(body);
188-
if (note) { commit.note = note; }
189-
}
190-
191153
// if the subject ends in ' (#dddd)', treat it as a pull request id
192154
let match;
193155
if ((match = subject.match(/^(.*)\s\(#(\d+)\)$/))) {
@@ -219,7 +181,6 @@ const parseCommitMessage = (commitMessage, commit) => {
219181

220182
// https://help.github.com/articles/closing-issues-using-keywords/
221183
if ((match = body.match(/\b(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved|for)\s#(\d+)\b/i))) {
222-
commit.issueNumber = parseInt(match[1]);
223184
commit.semanticType = commit.semanticType || 'fix';
224185
}
225186

@@ -243,32 +204,32 @@ const parseCommitMessage = (commitMessage, commit) => {
243204
const parsePullText = (pull, commit) => parseCommitMessage(`${pull.data.title}\n\n${pull.data.body}`, commit);
244205

245206
const getLocalCommitHashes = async (dir, ref) => {
246-
const args = ['log', '-z', '--format=%H', ref];
247-
return (await runGit(dir, args)).split('\0').map(hash => hash.trim());
207+
const args = ['log', '--format=%H', ref];
208+
return (await runGit(dir, args)).split(/[\r\n]+/).map(hash => hash.trim());
248209
};
249210

250211
// return an array of Commits
251212
const getLocalCommits = async (module, point1, point2) => {
252213
const { owner, repo, dir } = module;
253214

254-
const fieldSep = '||';
255-
const format = ['%H', '%B'].join(fieldSep);
256-
const args = ['log', '-z', '--cherry-pick', '--right-only', '--first-parent', `--format=${format}`, `${point1}..${point2}`];
257-
const logs = (await runGit(dir, args)).split('\0').map(field => field.trim());
215+
const fieldSep = ',';
216+
const format = ['%H', '%s'].join(fieldSep);
217+
const args = ['log', '--cherry-pick', '--right-only', '--first-parent', `--format=${format}`, `${point1}..${point2}`];
218+
const logs = (await runGit(dir, args)).split(/[\r\n]+/).map(field => field.trim());
258219

259220
const commits = [];
260221
for (const log of logs) {
261222
if (!log) {
262223
continue;
263224
}
264-
const [hash, message] = log.split(fieldSep, 2).map(field => field.trim());
265-
commits.push(parseCommitMessage(message, new Commit(hash, owner, repo)));
225+
const [hash, subject] = log.split(fieldSep, 2).map(field => field.trim());
226+
commits.push(parseCommitMessage(subject, new Commit(hash, owner, repo)));
266227
}
267228
return commits;
268229
};
269230

270231
const checkCache = async (name, operation) => {
271-
const filename = path.resolve(CACHE_DIR, name);
232+
const filename = path.resolve(getCacheDir(), name);
272233
if (fs.existsSync(filename)) {
273234
return JSON.parse(fs.readFileSync(filename, 'utf8'));
274235
}
@@ -292,15 +253,16 @@ async function runRetryable (fn, maxRetries) {
292253
}
293254
}
294255
// Silently eat 404s.
295-
if (lastError.status !== 404) throw lastError;
256+
// Silently eat 422s, which come from "No commit found for SHA"
257+
if (lastError.status !== 404 && lastError.status !== 422) throw lastError;
296258
}
297259

298260
const getPullCacheFilename = ghKey => `${ghKey.owner}-${ghKey.repo}-pull-${ghKey.number}`;
299261

300262
const getCommitPulls = async (owner, repo, hash) => {
301263
const name = `${owner}-${repo}-commit-${hash}`;
302264
const retryableFunc = () => octokit.repos.listPullRequestsAssociatedWithCommit({ owner, repo, commit_sha: hash });
303-
const ret = await checkCache(name, () => runRetryable(retryableFunc, MAX_FAIL_COUNT));
265+
let ret = await checkCache(name, () => runRetryable(retryableFunc, MAX_FAIL_COUNT));
304266

305267
// only merged pulls belong in release notes
306268
if (ret && ret.data) {
@@ -316,6 +278,11 @@ const getCommitPulls = async (owner, repo, hash) => {
316278
}
317279
}
318280

281+
// ensure the return value has the expected structure, even on failure
282+
if (!ret || !ret.data) {
283+
ret = { data: [] };
284+
}
285+
319286
return ret;
320287
};
321288

@@ -447,19 +414,25 @@ function getOldestMajorBranchOfCommit (commit, pool) {
447414
.shift();
448415
}
449416

417+
function commitExistsBeforeMajor (commit, pool, major) {
418+
const firstAppearance = getOldestMajorBranchOfCommit(commit, pool);
419+
return firstAppearance && (firstAppearance < major);
420+
}
421+
450422
/***
451423
**** Main
452424
***/
453425

454426
const getNotes = async (fromRef, toRef, newVersion) => {
455-
if (!fs.existsSync(CACHE_DIR)) {
456-
fs.mkdirSync(CACHE_DIR);
427+
const cacheDir = getCacheDir();
428+
if (!fs.existsSync(cacheDir)) {
429+
fs.mkdirSync(cacheDir);
457430
}
458431

459432
const pool = new Pool();
460433

461434
// get the electron/electron commits
462-
const electron = { owner: 'electron', repo: 'electron', dir: ELECTRON_VERSION };
435+
const electron = { owner: 'electron', repo: 'electron', dir: path.resolve(SRC_DIR, 'electron') };
463436
await addRepoToPool(pool, electron, fromRef, toRef);
464437

465438
// Don't include submodules if comparing across major versions;
@@ -496,27 +469,24 @@ const getNotes = async (fromRef, toRef, newVersion) => {
496469
// ensure the commit has a note
497470
for (const commit of pool.commits) {
498471
for (const prKey of commit.prKeys.values()) {
499-
commit.note = commit.note || await getNoteFromClerk(prKey);
500472
if (commit.note) {
501473
break;
502474
}
475+
commit.note = await getNoteFromClerk(prKey);
503476
}
504-
// use a fallback note in case someone missed a 'Notes' comment
505-
commit.note = commit.note || commit.subject;
506477
}
507478

508479
// remove non-user-facing commits
509480
pool.commits = pool.commits
510-
.filter(commit => commit.note !== NO_NOTES)
481+
.filter(commit => commit.note && (commit.note !== NO_NOTES))
511482
.filter(commit => !((commit.note || commit.subject).match(/^[Bb]ump v\d+\.\d+\.\d+/)));
512483

513484
if (!shouldIncludeMultibranchChanges(newVersion)) {
514-
const currentMajor = semver.parse(newVersion).major;
515-
pool.commits = pool.commits
516-
.filter(commit => getOldestMajorBranchOfCommit(commit, pool) >= currentMajor);
485+
const { major } = semver.parse(newVersion);
486+
pool.commits = pool.commits.filter(commit => !commitExistsBeforeMajor(commit, pool, major));
517487
}
518488

519-
pool.commits = removeSupercededChromiumUpdates(pool.commits);
489+
pool.commits = removeSupercededStackUpdates(pool.commits);
520490

521491
const notes = {
522492
breaking: [],
@@ -550,18 +520,24 @@ const getNotes = async (fromRef, toRef, newVersion) => {
550520
return notes;
551521
};
552522

553-
const removeSupercededChromiumUpdates = (commits) => {
554-
const chromiumRegex = /^Updated Chromium to \d+\.\d+\.\d+\.\d+/;
555-
const updates = commits.filter(commit => (commit.note || commit.subject).match(chromiumRegex));
556-
const keepers = commits.filter(commit => !updates.includes(commit));
523+
const removeSupercededStackUpdates = (commits) => {
524+
const updateRegex = /^Updated ([a-zA-Z.]+) to v?([\d.]+)/;
525+
const notupdates = [];
557526

558-
// keep the newest update.
559-
if (updates.length) {
560-
const compare = (a, b) => (a.note || a.subject).localeCompare(b.note || b.subject);
561-
keepers.push(updates.sort(compare).pop());
527+
const newest = {};
528+
for (const commit of commits) {
529+
const match = (commit.note || commit.subject).match(updateRegex);
530+
if (!match) {
531+
notupdates.push(commit);
532+
continue;
533+
}
534+
const [ , dep, version ] = match;
535+
if (!newest[dep] || newest[dep].version < version) {
536+
newest[dep] = { commit, version };
537+
}
562538
}
563539

564-
return keepers;
540+
return [ ...notupdates, ...Object.values(newest).map(o => o.commit) ];
565541
};
566542

567543
/***

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-0600420bac25439fc2067d51c6aaa4ee11770577‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-2955c67c4ea712fa22773ac9113709fc952bfd49‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-2fad53e66b1a2cb6f7dad88fe9bb62d7a461fe98‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-467409458e716c68b35fa935d556050ca6bed1c4‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-89eb309d0b22bd4aec058ffaf983e81e56a5c378‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-8bc0c92137f4a77dc831ca644a86a3e48b51a11e‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-commit-a6ff42c190cb5caf8f3e217748e49183a951491b‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-issue-20214-comments‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎spec-main/fixtures/release-notes/cache/electron-electron-issue-21891-comments‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"status":200,"url":"https://api.github.com/repos/electron/electron/issues/21891/comments?per_page=100","headers":{"access-control-allow-origin":"*","access-control-expose-headers":"ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset","cache-control":"private, max-age=60, s-maxage=60","connection":"close","content-encoding":"gzip","content-security-policy":"default-src 'none'","content-type":"application/json; charset=utf-8","date":"Tue, 26 May 2020 04:07:22 GMT","etag":"W/\"f3361c5c493edbcc6774be228131a636\"","referrer-policy":"origin-when-cross-origin, strict-origin-when-cross-origin","server":"GitHub.com","status":"200 OK","strict-transport-security":"max-age=31536000; includeSubdomains; preload","transfer-encoding":"chunked","vary":"Accept, Authorization, Cookie, X-GitHub-OTP, Accept-Encoding, Accept, X-Requested-With","x-accepted-oauth-scopes":"","x-content-type-options":"nosniff","x-frame-options":"deny","x-github-media-type":"github.v3; format=json","x-github-request-id":"8F4E:231E:52E8BF:8AF8CD:5ECC95F9","x-oauth-scopes":"repo","x-ratelimit-limit":"5000","x-ratelimit-remaining":"4997","x-ratelimit-reset":"1590469446","x-xss-protection":"1; mode=block"},"data":[{"url":"https://api.github.com/repos/electron/electron/issues/comments/579570143","html_url":"https://github.com/electron/electron/pull/21891#issuecomment-579570143","issue_url":"https://api.github.com/repos/electron/electron/issues/21891","id":579570143,"node_id":"MDEyOklzc3VlQ29tbWVudDU3OTU3MDE0Mw==","user":{"login":"bitdisaster","id":5191943,"node_id":"MDQ6VXNlcjUxOTE5NDM=","avatar_url":"https://avatars3.githubusercontent.com/u/5191943?v=4","gravatar_id":"","url":"https://api.github.com/users/bitdisaster","html_url":"https://github.com/bitdisaster","followers_url":"https://api.github.com/users/bitdisaster/followers","following_url":"https://api.github.com/users/bitdisaster/following{/other_user}","gists_url":"https://api.github.com/users/bitdisaster/gists{/gist_id}","starred_url":"https://api.github.com/users/bitdisaster/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bitdisaster/subscriptions","organizations_url":"https://api.github.com/users/bitdisaster/orgs","repos_url":"https://api.github.com/users/bitdisaster/repos","events_url":"https://api.github.com/users/bitdisaster/events{/privacy}","received_events_url":"https://api.github.com/users/bitdisaster/received_events","type":"User","site_admin":false},"created_at":"2020-01-29T02:58:25Z","updated_at":"2020-01-29T02:58:25Z","author_association":"MEMBER","body":"@zcbenz I solved the mac/linux problem a bit differently. @MarshallOfSound recommended the use of converter to me and I like the approach. Does the typedef via conditional compiling work for you to?"},{"url":"https://api.github.com/repos/electron/electron/issues/comments/580589854","html_url":"https://github.com/electron/electron/pull/21891#issuecomment-580589854","issue_url":"https://api.github.com/repos/electron/electron/issues/21891","id":580589854,"node_id":"MDEyOklzc3VlQ29tbWVudDU4MDU4OTg1NA==","user":{"login":"release-clerk[bot]","id":42386326,"node_id":"MDM6Qm90NDIzODYzMjY=","avatar_url":"https://avatars0.githubusercontent.com/in/16104?v=4","gravatar_id":"","url":"https://api.github.com/users/release-clerk%5Bbot%5D","html_url":"https://github.com/apps/release-clerk","followers_url":"https://api.github.com/users/release-clerk%5Bbot%5D/followers","following_url":"https://api.github.com/users/release-clerk%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/release-clerk%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/release-clerk%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/release-clerk%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/release-clerk%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/release-clerk%5Bbot%5D/repos","events_url":"https://api.github.com/users/release-clerk%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/release-clerk%5Bbot%5D/received_events","type":"Bot","site_admin":false},"created_at":"2020-01-31T05:37:07Z","updated_at":"2020-01-31T05:37:07Z","author_association":"NONE","body":"**Release Notes Persisted**\n\n> Added GUID parameter to Tray API to avoid system tray icon demotion on Windows "}]}

0 commit comments

Comments
 (0)