Browse Source

chore: sync 8-x-y release notes script to master (#25306)

* adds the '(Also in N-x-y)' annotations
* handle sublists in release notes (#25279)
* has prepare-release.js catch thrown exceptions (#24923)
* syncs related tests
Charles Kerr 4 years ago
parent
commit
32c7b35da6
34 changed files with 712 additions and 428 deletions
  1. 42 28
      script/release/notes/index.js
  2. 334 399
      script/release/notes/notes.js
  3. 5 1
      script/release/prepare-release.js
  4. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-0600420bac25439fc2067d51c6aaa4ee11770577
  5. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-2955c67c4ea712fa22773ac9113709fc952bfd49
  6. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-2fad53e66b1a2cb6f7dad88fe9bb62d7a461fe98
  7. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-467409458e716c68b35fa935d556050ca6bed1c4
  8. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-61dc1c88fd34a3e8fff80c80ed79d0455970e610
  9. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-89eb309d0b22bd4aec058ffaf983e81e56a5c378
  10. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-8bc0c92137f4a77dc831ca644a86a3e48b51a11e
  11. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-commit-a6ff42c190cb5caf8f3e217748e49183a951491b
  12. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-20214-comments
  13. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-21497-comments
  14. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-21891-comments
  15. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-21946-comments
  16. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-22750-comments
  17. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-22770-comments
  18. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-22828-comments
  19. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-25052-comments
  20. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-issue-25216-comments
  21. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-20214
  22. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-20620
  23. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-21497
  24. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-21591
  25. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-21891
  26. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-21946
  27. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-22750
  28. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-22770
  29. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-22828
  30. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-25052
  31. 0 0
      spec-main/fixtures/release-notes/cache/electron-electron-pull-25216
  32. 2 0
      spec-main/package.json
  33. 213 0
      spec-main/release-notes-spec.ts
  34. 116 0
      spec-main/yarn.lock

+ 42 - 28
script/release/notes/index.js

@@ -8,9 +8,10 @@ const semver = require('semver');
 const { ELECTRON_DIR } = require('../../lib/utils');
 const notesGenerator = require('./notes.js');
 
-const semverify = version => version.replace(/^origin\//, '').replace('x', '0').replace(/-/g, '.');
+const semverify = version => version.replace(/^origin\//, '').replace(/[xy]/g, '0').replace(/-/g, '.');
 
 const runGit = async (args) => {
+  console.info(`Running: git ${args.join(' ')}`);
   const response = await GitProcess.exec(args, ELECTRON_DIR);
   if (response.exitCode !== 0) {
     throw new Error(response.stderr.trim());
@@ -19,15 +20,20 @@ const runGit = async (args) => {
 };
 
 const tagIsSupported = tag => tag && !tag.includes('nightly') && !tag.includes('unsupported');
-const tagIsBeta = tag => tag.includes('beta');
+const tagIsBeta = tag => tag && tag.includes('beta');
 const tagIsStable = tag => tagIsSupported(tag) && !tagIsBeta(tag);
 
 const getTagsOf = async (point) => {
-  return (await runGit(['tag', '--merged', point]))
-    .split('\n')
-    .map(tag => tag.trim())
-    .filter(tag => semver.valid(tag))
-    .sort(semver.compare);
+  try {
+    const tags = await runGit(['tag', '--merged', point]);
+    return tags.split('\n')
+      .map(tag => tag.trim())
+      .filter(tag => semver.valid(tag))
+      .sort(semver.compare);
+  } catch (err) {
+    console.error(`Failed to fetch tags for point ${point}`);
+    throw err;
+  }
 };
 
 const getTagsOnBranch = async (point) => {
@@ -41,26 +47,36 @@ const getTagsOnBranch = async (point) => {
 };
 
 const getBranchOf = async (point) => {
-  const branches = (await runGit(['branch', '-a', '--contains', point]))
-    .split('\n')
-    .map(branch => branch.trim())
-    .filter(branch => !!branch);
-  const current = branches.find(branch => branch.startsWith('* '));
-  return current ? current.slice(2) : branches.shift();
+  try {
+    const branches = (await runGit(['branch', '-a', '--contains', point]))
+      .split('\n')
+      .map(branch => branch.trim())
+      .filter(branch => !!branch);
+    const current = branches.find(branch => branch.startsWith('* '));
+    return current ? current.slice(2) : branches.shift();
+  } catch (err) {
+    console.error(`Failed to fetch branch for ${point}: `, err);
+    throw err;
+  }
 };
 
 const getAllBranches = async () => {
-  return (await runGit(['branch', '--remote']))
-    .split('\n')
-    .map(branch => branch.trim())
-    .filter(branch => !!branch)
-    .filter(branch => branch !== 'origin/HEAD -> origin/master')
-    .sort();
+  try {
+    const branches = await runGit(['branch', '--remote']);
+    return branches.split('\n')
+      .map(branch => branch.trim())
+      .filter(branch => !!branch)
+      .filter(branch => branch !== 'origin/HEAD -> origin/master')
+      .sort();
+  } catch (err) {
+    console.error('Failed to fetch all branches');
+    throw err;
+  }
 };
 
 const getStabilizationBranches = async () => {
   return (await getAllBranches())
-    .filter(branch => /^origin\/\d+-\d+-x$/.test(branch));
+    .filter(branch => /^origin\/\d+-\d+-x$/.test(branch) || /^origin\/\d+-x-y$/.test(branch));
 };
 
 const getPreviousStabilizationBranch = async (current) => {
@@ -120,7 +136,7 @@ const getPreviousPoint = async (point) => {
   }
 };
 
-async function getReleaseNotes (range, newVersion, explicitLinks) {
+async function getReleaseNotes (range, newVersion) {
   const rangeList = range.split('..') || ['HEAD'];
   const to = rangeList.pop();
   const from = rangeList.pop() || (await getPreviousPoint(to));
@@ -129,10 +145,9 @@ async function getReleaseNotes (range, newVersion, explicitLinks) {
     newVersion = to;
   }
 
-  console.log(`Generating release notes between ${from} and ${to} for version ${newVersion}`);
   const notes = await notesGenerator.get(from, to, newVersion);
   const ret = {
-    text: notesGenerator.render(notes, explicitLinks)
+    text: notesGenerator.render(notes)
   };
 
   if (notes.unknown.length) {
@@ -144,8 +159,8 @@ async function getReleaseNotes (range, newVersion, explicitLinks) {
 
 async function main () {
   const opts = minimist(process.argv.slice(2), {
-    boolean: [ 'explicit-links', 'help' ],
-    string: [ 'version' ]
+    boolean: ['help'],
+    string: ['version']
   });
   opts.range = opts._.shift();
   if (opts.help || !opts.range) {
@@ -153,14 +168,13 @@ async function main () {
     console.log(`
 easy usage: ${name} version
 
-full usage: ${name} [begin..]end [--version version] [--explicit-links]
+full usage: ${name} [begin..]end [--version version]
 
  * 'begin' and 'end' are two git references -- tags, branches, etc --
    from which the release notes are generated.
  * if omitted, 'begin' defaults to the previous tag in end's branch.
  * if omitted, 'version' defaults to 'end'. Specifying a version is
    useful if you're making notes on a new version that isn't tagged yet.
- * 'explicit-links' makes every note's issue, commit, or pull an MD link
 
 For example, these invocations are equivalent:
   ${process.argv[1]} v4.0.1
@@ -169,7 +183,7 @@ For example, these invocations are equivalent:
     return 0;
   }
 
-  const notes = await getReleaseNotes(opts.range, opts.version, opts['explicit-links']);
+  const notes = await getReleaseNotes(opts.range, opts.version);
   console.log(notes.text);
   if (notes.warning) {
     throw new Error(notes.warning);

+ 334 - 399
script/release/notes/notes.js

@@ -1,31 +1,84 @@
 #!/usr/bin/env node
 
-const childProcess = require('child_process');
+'use strict';
+
 const fs = require('fs');
-const os = require('os');
 const path = require('path');
 
 const { GitProcess } = require('dugite');
+
 const octokit = require('@octokit/rest')({
   auth: process.env.ELECTRON_GITHUB_TOKEN
 });
-const semver = require('semver');
 
-const { ELECTRON_VERSION, SRC_DIR } = require('../../lib/utils');
+const { ELECTRON_DIR } = require('../../lib/utils');
 
 const MAX_FAIL_COUNT = 3;
 const CHECK_INTERVAL = 5000;
 
-const CACHE_DIR = path.resolve(__dirname, '.cache');
+const TROP_LOGIN = 'trop[bot]';
+
 const NO_NOTES = 'No notes';
-const FOLLOW_REPOS = [ 'electron/electron', 'electron/node' ];
 
-const breakTypes = new Set(['breaking-change']);
 const docTypes = new Set(['doc', 'docs']);
 const featTypes = new Set(['feat', 'feature']);
 const fixTypes = new Set(['fix']);
 const otherTypes = new Set(['spec', 'build', 'test', 'chore', 'deps', 'refactor', 'tools', 'vendor', 'perf', 'style', 'ci']);
-const knownTypes = new Set([...breakTypes.keys(), ...docTypes.keys(), ...featTypes.keys(), ...fixTypes.keys(), ...otherTypes.keys()]);
+const knownTypes = new Set([...docTypes.keys(), ...featTypes.keys(), ...fixTypes.keys(), ...otherTypes.keys()]);
+
+const getCacheDir = () => process.env.NOTES_CACHE_PATH || path.resolve(__dirname, '.cache');
+
+/**
+***
+**/
+
+// link to a GitHub item, e.g. an issue or pull request
+class GHKey {
+  constructor (owner, repo, number) {
+    this.owner = owner;
+    this.repo = repo;
+    this.number = number;
+  }
+
+  static NewFromPull (pull) {
+    const owner = pull.base.repo.owner.login;
+    const repo = pull.base.repo.name;
+    const number = pull.number;
+    return new GHKey(owner, repo, number);
+  }
+}
+
+class Commit {
+  constructor (hash, owner, repo) {
+    this.hash = hash; // string
+    this.owner = owner; // string
+    this.repo = repo; // string
+
+    this.isBreakingChange = false;
+    this.note = null; // string
+
+    // A set of branches to which this change has been merged.
+    // '8-x-y' => GHKey { owner: 'electron', repo: 'electron', number: 23714 }
+    this.trops = new Map(); // Map<string,GHKey>
+
+    this.prKeys = new Set(); // GHKey
+    this.revertHash = null; // string
+    this.semanticType = null; // string
+    this.subject = null; // string
+  }
+}
+
+class Pool {
+  constructor () {
+    this.commits = []; // Array<Commit>
+    this.processedHashes = new Set();
+    this.pulls = {}; // GHKey.number => octokit pull object
+  }
+}
+
+/**
+***
+**/
 
 const runGit = async (dir, args) => {
   const response = await GitProcess.exec(args, dir);
@@ -39,24 +92,8 @@ const getCommonAncestor = async (dir, point1, point2) => {
   return runGit(dir, ['merge-base', point1, point2]);
 };
 
-const setPullRequest = (commit, owner, repo, number) => {
-  if (!owner || !repo || !number) {
-    throw new Error(JSON.stringify({ owner, repo, number }, null, 2));
-  }
-
-  if (!commit.originalPr) {
-    commit.originalPr = commit.pr;
-  }
-
-  commit.pr = { owner, repo, number };
-
-  if (!commit.originalPr) {
-    commit.originalPr = commit.pr;
-  }
-};
-
-const getNoteFromClerk = async (number, owner, repo) => {
-  const comments = await getComments(number, owner, repo);
+const getNoteFromClerk = async (ghKey) => {
+  const comments = await getComments(ghKey);
   if (!comments || !comments.data) return;
 
   const CLERK_LOGIN = 'release-clerk[bot]';
@@ -72,71 +109,39 @@ const getNoteFromClerk = async (number, owner, repo) => {
       return NO_NOTES;
     }
     if (comment.body.startsWith(PERSIST_LEAD)) {
-      return comment.body
+      let lines = comment.body
         .slice(PERSIST_LEAD.length).trim() // remove PERSIST_LEAD
-        .split('\r?\n') // break into lines
+        .split(/\r?\n/) // split into lines
         .map(line => line.trim())
         .filter(line => line.startsWith(QUOTE_LEAD)) // notes are quoted
-        .map(line => line.slice(QUOTE_LEAD.length)) // unquote the lines
-        .join(' ') // join the note lines
+        .map(line => line.slice(QUOTE_LEAD.length)); // unquote the lines
+
+      const firstLine = lines.shift();
+      // indent anything after the first line to ensure that
+      // multiline notes with their own sub-lists don't get
+      // parsed in the markdown as part of the top-level list
+      // (example: https://github.com/electron/electron/pull/25216)
+      lines = lines.map(line => '  ' + line);
+      return [firstLine, ...lines]
+        .join('\n') // join the lines
         .trim();
     }
   }
 };
 
-// copied from https://github.com/electron/clerk/blob/master/src/index.ts#L4-L13
-const OMIT_FROM_RELEASE_NOTES_KEYS = [
-  'no-notes',
-  'no notes',
-  'no_notes',
-  'none',
-  'no',
-  'nothing',
-  'empty',
-  'blank'
-];
-
-const getNoteFromBody = body => {
-  if (!body) {
-    return null;
-  }
-
-  const NOTE_PREFIX = 'Notes: ';
-  const NOTE_HEADER = '#### Release Notes';
-
-  let note = body
-    .split(/\r?\n\r?\n/) // split into paragraphs
-    .map(paragraph => paragraph.trim())
-    .map(paragraph => paragraph.startsWith(NOTE_HEADER) ? paragraph.slice(NOTE_HEADER.length).trim() : paragraph)
-    .find(paragraph => paragraph.startsWith(NOTE_PREFIX));
-
-  if (note) {
-    note = note
-      .slice(NOTE_PREFIX.length)
-      .replace(/<!--.*-->/, '') // '<!-- change summary here-->'
-      .replace(/\r?\n/, ' ') // remove newlines
-      .trim();
-  }
-
-  if (note && OMIT_FROM_RELEASE_NOTES_KEYS.includes(note.toLowerCase())) {
-    return NO_NOTES;
-  }
-
-  return note;
-};
-
 /**
  * Looks for our project's conventions in the commit message:
  *
- * 'semantic: some description' -- sets type, subject
+ * 'semantic: some description' -- sets semanticType, subject
  * 'some description (#99999)' -- sets subject, pr
- * 'Fixes #3333' -- sets issueNumber
  * 'Merge pull request #99999 from ${branchname}' -- sets pr
  * 'This reverts commit ${sha}' -- sets revertHash
- * line starting with 'BREAKING CHANGE' in body -- sets breakingChange
+ * line starting with 'BREAKING CHANGE' in body -- sets isBreakingChange
  * 'Backport of #99999' -- sets pr
  */
-const parseCommitMessage = (commitMessage, owner, repo, commit = {}) => {
+const parseCommitMessage = (commitMessage, commit) => {
+  const { owner, repo } = commit;
+
   // split commitMessage into subject & body
   let subject = commitMessage;
   let body = '';
@@ -146,64 +151,38 @@ const parseCommitMessage = (commitMessage, owner, repo, commit = {}) => {
     subject = subject.slice(0, pos).trim();
   }
 
-  if (!commit.originalSubject) {
-    commit.originalSubject = subject;
-  }
-
-  if (body) {
-    commit.body = body;
-
-    const note = getNoteFromBody(body);
-    if (note) { commit.note = note; }
-  }
-
   // if the subject ends in ' (#dddd)', treat it as a pull request id
   let match;
   if ((match = subject.match(/^(.*)\s\(#(\d+)\)$/))) {
-    setPullRequest(commit, owner, repo, parseInt(match[2]));
+    commit.prKeys.add(new GHKey(owner, repo, parseInt(match[2])));
     subject = match[1];
   }
 
   // if the subject begins with 'word:', treat it as a semantic commit
   if ((match = subject.match(/^(\w+):\s(.*)$/))) {
-    const type = match[1].toLocaleLowerCase();
-    if (knownTypes.has(type)) {
-      commit.type = type;
+    const semanticType = match[1].toLocaleLowerCase();
+    if (knownTypes.has(semanticType)) {
+      commit.semanticType = semanticType;
       subject = match[2];
     }
   }
 
   // Check for GitHub commit message that indicates a PR
   if ((match = subject.match(/^Merge pull request #(\d+) from (.*)$/))) {
-    setPullRequest(commit, owner, repo, parseInt(match[1]));
-    commit.pr.branch = match[2].trim();
+    commit.prKeys.add(new GHKey(owner, repo, parseInt(match[1])));
   }
 
-  // Check for a trop comment that indicates a PR
-  if ((match = commitMessage.match(/\bBackport of #(\d+)\b/))) {
-    setPullRequest(commit, owner, repo, parseInt(match[1]));
+  // Check for a comment that indicates a PR
+  const backportPattern = /(?:^|\n)(?:manual |manually )?backport.*(?:#(\d+)|\/pull\/(\d+))/im;
+  if ((match = commitMessage.match(backportPattern))) {
+    // This might be the first or second capture group depending on if it's a link or not.
+    const backportNumber = match[1] ? parseInt(match[1], 10) : parseInt(match[2], 10);
+    commit.prKeys.add(new GHKey(owner, repo, backportNumber));
   }
 
   // https://help.github.com/articles/closing-issues-using-keywords/
-  if ((match = subject.match(/\b(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved|for)\s#(\d+)\b/))) {
-    commit.issueNumber = parseInt(match[1]);
-    if (!commit.type) {
-      commit.type = 'fix';
-    }
-  }
-
-  // look for 'fixes' in markdown; e.g. 'Fixes [#8952](https://github.com/electron/electron/issues/8952)'
-  if (!commit.issueNumber && ((match = commitMessage.match(/Fixes \[#(\d+)\]\(https:\/\/github.com\/(\w+)\/(\w+)\/issues\/(\d+)\)/)))) {
-    commit.issueNumber = parseInt(match[1]);
-    if (commit.pr && commit.pr.number === commit.issueNumber) {
-      commit.pr = null;
-    }
-    if (commit.originalPr && commit.originalPr.number === commit.issueNumber) {
-      commit.originalPr = null;
-    }
-    if (!commit.type) {
-      commit.type = 'fix';
-    }
+  if ((match = body.match(/\b(?:close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved|for)\s#(\d+)\b/i))) {
+    commit.semanticType = commit.semanticType || 'fix';
   }
 
   // https://www.conventionalcommits.org/en
@@ -211,7 +190,7 @@ const parseCommitMessage = (commitMessage, owner, repo, commit = {}) => {
     .split(/\r?\n/) // split into lines
     .map(line => line.trim())
     .some(line => line.startsWith('BREAKING CHANGE'))) {
-    commit.type = 'breaking-change';
+    commit.isBreakingChange = true;
   }
 
   // Check for a reversion commit
@@ -219,82 +198,47 @@ const parseCommitMessage = (commitMessage, owner, repo, commit = {}) => {
     commit.revertHash = match[1];
   }
 
-  // Edge case: manual backport where commit has `owner/repo#pull` notation
-  if (commitMessage.toLowerCase().includes('backport') &&
-      ((match = commitMessage.match(/\b(\w+)\/(\w+)#(\d+)\b/)))) {
-    const [ , owner, repo, number ] = match;
-    if (FOLLOW_REPOS.includes(`${owner}/${repo}`)) {
-      setPullRequest(commit, owner, repo, number);
-    }
-  }
-
-  // Edge case: manual backport where commit has a link to the backport PR
-  if (commitMessage.includes('ackport') &&
-      ((match = commitMessage.match(/https:\/\/github\.com\/(\w+)\/(\w+)\/pull\/(\d+)/)))) {
-    const [ , owner, repo, number ] = match;
-    if (FOLLOW_REPOS.includes(`${owner}/${repo}`)) {
-      setPullRequest(commit, owner, repo, number);
-    }
-  }
-
-  // Legacy commits: pre-semantic commits
-  if (!commit.type || commit.type === 'chore') {
-    const commitMessageLC = commitMessage.toLocaleLowerCase();
-    if ((match = commitMessageLC.match(/\bchore\((\w+)\):/))) {
-      // example: 'Chore(docs): description'
-      commit.type = knownTypes.has(match[1]) ? match[1] : 'chore';
-    } else if (commitMessageLC.match(/\b(?:fix|fixes|fixed)/)) {
-      // example: 'fix a bug'
-      commit.type = 'fix';
-    } else if (commitMessageLC.match(/\[(?:docs|doc)\]/)) {
-      // example: '[docs]
-      commit.type = 'doc';
-    }
-  }
-
   commit.subject = subject.trim();
   return commit;
 };
 
+const parsePullText = (pull, commit) => parseCommitMessage(`${pull.data.title}\n\n${pull.data.body}`, commit);
+
 const getLocalCommitHashes = async (dir, ref) => {
-  const args = ['log', '-z', `--format=%H`, ref];
-  return (await runGit(dir, args)).split(`\0`).map(hash => hash.trim());
+  const args = ['log', '--format=%H', ref];
+  return (await runGit(dir, args))
+    .split(/\r?\n/) // split into lines
+    .map(hash => hash.trim());
 };
 
-/*
- * possible properties:
- * breakingChange, email, hash, issueNumber, originalSubject, parentHashes,
- * pr { owner, repo, number, branch }, revertHash, subject, type
- */
-const getLocalCommitDetails = async (module, point1, point2) => {
+// return an array of Commits
+const getLocalCommits = async (module, point1, point2) => {
   const { owner, repo, dir } = module;
 
-  const fieldSep = '||';
-  const format = ['%H', '%P', '%aE', '%B'].join(fieldSep);
-  const args = ['log', '-z', '--cherry-pick', '--right-only', '--first-parent', `--format=${format}`, `${point1}..${point2}`];
-  const commits = (await runGit(dir, args)).split(`\0`).map(field => field.trim());
-  const details = [];
-  for (const commit of commits) {
-    if (!commit) {
+  const fieldSep = ',';
+  const format = ['%H', '%s'].join(fieldSep);
+  const args = ['log', '--cherry-pick', '--right-only', '--first-parent', `--format=${format}`, `${point1}..${point2}`];
+  const logs = (await runGit(dir, args))
+    .split(/\r?\n/) // split into lines
+    .map(field => field.trim());
+
+  const commits = [];
+  for (const log of logs) {
+    if (!log) {
       continue;
     }
-    const [ hash, parentHashes, email, commitMessage ] = commit.split(fieldSep, 4).map(field => field.trim());
-    details.push(parseCommitMessage(commitMessage, owner, repo, {
-      email,
-      hash,
-      owner,
-      repo,
-      parentHashes: parentHashes.split()
-    }));
-  }
-  return details;
+    const [hash, subject] = log.split(fieldSep, 2).map(field => field.trim());
+    commits.push(parseCommitMessage(subject, new Commit(hash, owner, repo)));
+  }
+  return commits;
 };
 
 const checkCache = async (name, operation) => {
-  const filename = path.resolve(CACHE_DIR, name);
+  const filename = path.resolve(getCacheDir(), name);
   if (fs.existsSync(filename)) {
     return JSON.parse(fs.readFileSync(filename, 'utf8'));
   }
+  process.stdout.write('.');
   const response = await operation();
   if (response) {
     fs.writeFileSync(filename, JSON.stringify(response));
@@ -314,16 +258,48 @@ async function runRetryable (fn, maxRetries) {
     }
   }
   // Silently eat 404s.
-  if (lastError.status !== 404) throw lastError;
+  // Silently eat 422s, which come from "No commit found for SHA"
+  if (lastError.status !== 404 && lastError.status !== 422) throw lastError;
 }
 
-const getPullRequest = async (number, owner, repo) => {
-  const name = `${owner}-${repo}-pull-${number}`;
+const getPullCacheFilename = ghKey => `${ghKey.owner}-${ghKey.repo}-pull-${ghKey.number}`;
+
+const getCommitPulls = async (owner, repo, hash) => {
+  const name = `${owner}-${repo}-commit-${hash}`;
+  const retryableFunc = () => octokit.repos.listPullRequestsAssociatedWithCommit({ owner, repo, commit_sha: hash });
+  let ret = await checkCache(name, () => runRetryable(retryableFunc, MAX_FAIL_COUNT));
+
+  // only merged pulls belong in release notes
+  if (ret && ret.data) {
+    ret.data = ret.data.filter(pull => pull.merged_at);
+  }
+
+  // cache the pulls
+  if (ret && ret.data) {
+    for (const pull of ret.data) {
+      const cachefile = getPullCacheFilename(GHKey.NewFromPull(pull));
+      const payload = { ...ret, data: pull };
+      await checkCache(cachefile, () => payload);
+    }
+  }
+
+  // ensure the return value has the expected structure, even on failure
+  if (!ret || !ret.data) {
+    ret = { data: [] };
+  }
+
+  return ret;
+};
+
+const getPullRequest = async (ghKey) => {
+  const { number, owner, repo } = ghKey;
+  const name = getPullCacheFilename(ghKey);
   const retryableFunc = () => octokit.pulls.get({ pull_number: number, owner, repo });
   return checkCache(name, () => runRetryable(retryableFunc, MAX_FAIL_COUNT));
 };
 
-const getComments = async (number, owner, repo) => {
+const getComments = async (ghKey) => {
+  const { number, owner, repo } = ghKey;
   const name = `${owner}-${repo}-issue-${number}-comments`;
   const retryableFunc = () => octokit.issues.listComments({ issue_number: number, owner, repo, per_page: 100 });
   return checkCache(name, () => runRetryable(retryableFunc, MAX_FAIL_COUNT));
@@ -331,103 +307,102 @@ const getComments = async (number, owner, repo) => {
 
 const addRepoToPool = async (pool, repo, from, to) => {
   const commonAncestor = await getCommonAncestor(repo.dir, from, to);
-  const oldHashes = await getLocalCommitHashes(repo.dir, from);
-  oldHashes.forEach(hash => { pool.processedHashes.add(hash); });
-  const commits = await getLocalCommitDetails(repo, commonAncestor, to);
-  pool.commits.push(...commits);
-};
 
-/***
-****  Other Repos
-***/
-
-// other repos - gn
+  // mark the old branch's commits as old news
+  for (const oldHash of await getLocalCommitHashes(repo.dir, from)) {
+    pool.processedHashes.add(oldHash);
+  }
 
-const getDepsVariable = async (ref, key) => {
-  // get a copy of that reference point's DEPS file
-  const deps = await runGit(ELECTRON_VERSION, ['show', `${ref}:DEPS`]);
-  const filename = path.resolve(os.tmpdir(), 'DEPS');
-  fs.writeFileSync(filename, deps);
+  // get the new branch's commits and the pulls associated with them
+  const commits = await getLocalCommits(repo, commonAncestor, to);
+  for (const commit of commits) {
+    const { owner, repo, hash } = commit;
+    for (const pull of (await getCommitPulls(owner, repo, hash)).data) {
+      commit.prKeys.add(GHKey.NewFromPull(pull));
+    }
+  }
 
-  // query the DEPS file
-  const response = childProcess.spawnSync(
-    'gclient',
-    ['getdep', '--deps-file', filename, '--var', key],
-    { encoding: 'utf8' }
-  );
+  pool.commits.push(...commits);
 
-  // cleanup
-  fs.unlinkSync(filename);
-  return response.stdout.trim();
+  // add the pulls
+  for (const commit of commits) {
+    let prKey;
+    for (prKey of commit.prKeys.values()) {
+      const pull = await getPullRequest(prKey);
+      if (!pull || !pull.data) continue; // couldn't get it
+      pool.pulls[prKey.number] = pull;
+      parsePullText(pull, commit);
+    }
+  }
 };
 
-const getDependencyCommitsGN = async (pool, fromRef, toRef) => {
-  const repos = [{ // just node
-    owner: 'electron',
-    repo: 'node',
-    dir: path.resolve(SRC_DIR, 'third_party', 'electron_node'),
-    deps_variable_name: 'node_version'
-  }];
-
-  for (const repo of repos) {
-    // the 'DEPS' file holds the dependency reference point
-    const key = repo.deps_variable_name;
-    const from = await getDepsVariable(fromRef, key);
-    const to = await getDepsVariable(toRef, key);
-    await addRepoToPool(pool, repo, from, to);
+// @return Map<string,GHKey>
+//   where the key is a branch name (e.g. '7-1-x' or '8-x-y')
+//   and the value is a GHKey to the PR
+async function getMergedTrops (commit, pool) {
+  const branches = new Map();
+
+  for (const prKey of commit.prKeys.values()) {
+    const pull = pool.pulls[prKey.number];
+    const mergedBranches = new Set(
+      ((pull && pull.data && pull.data.labels) ? pull.data.labels : [])
+        .map(label => ((label && label.name) ? label.name : '').match(/merged\/([0-9]+-[x0-9]-[xy0-9])/))
+        .filter(match => match)
+        .map(match => match[1])
+    );
+
+    if (mergedBranches.size > 0) {
+      const isTropComment = (comment) => comment && comment.user && comment.user.login === TROP_LOGIN;
+
+      const ghKey = GHKey.NewFromPull(pull.data);
+      const backportRegex = /backported this PR to "(.*)",\s+please check out #(\d+)/;
+      const getBranchNameAndPullKey = (comment) => {
+        const match = ((comment && comment.body) ? comment.body : '').match(backportRegex);
+        return match ? [match[1], new GHKey(ghKey.owner, ghKey.repo, parseInt(match[2]))] : null;
+      };
+
+      const comments = await getComments(ghKey);
+      ((comments && comments.data) ? comments.data : [])
+        .filter(isTropComment)
+        .map(getBranchNameAndPullKey)
+        .filter(pair => pair)
+        .filter(([branch]) => mergedBranches.has(branch))
+        .forEach(([branch, key]) => branches.set(branch, key));
+    }
   }
-};
 
-// Changes are interesting if they make a change relative to a previous
-// release in the same series. For example if you fix a Y.0.0 bug, that
-// should be included in the Y.0.1 notes even if it's also tropped back
-// to X.0.1.
-//
-// The phrase 'previous release' is important: if this is the first
-// prerelease or first stable release in a series, we omit previous
-// branches' changes. Otherwise we will have an overwhelmingly long
-// list of mostly-irrelevant changes.
-const shouldIncludeMultibranchChanges = (version) => {
-  let show = true;
-
-  if (semver.valid(version)) {
-    const prerelease = semver.prerelease(version);
-    show = prerelease
-      ? parseInt(prerelease.pop()) > 1
-      : semver.patch(version) > 0;
-  }
-
-  return show;
-};
+  return branches;
+}
+
+// @return the shorthand name of the branch that `ref` is on,
+//   e.g. a ref of '10.0.0-beta.1' will return '10-x-y'
+async function getBranchNameOfRef (ref, dir) {
+  return (await runGit(dir, ['branch', '--all', '--contains', ref, '--sort', 'version:refname']))
+    .split(/\r?\n/) // split into lines
+    .shift() // we sorted by refname and want the first result
+    .match(/(?:.*\/)?(.*)/)[1] // 'remote/origins/10-x-y' -> '10-x-y'
+    .trim();
+}
 
 /***
 ****  Main
 ***/
 
 const getNotes = async (fromRef, toRef, newVersion) => {
-  if (!fs.existsSync(CACHE_DIR)) {
-    fs.mkdirSync(CACHE_DIR);
+  const cacheDir = getCacheDir();
+  if (!fs.existsSync(cacheDir)) {
+    fs.mkdirSync(cacheDir);
   }
 
-  const pool = {
-    processedHashes: new Set(),
-    commits: []
-  };
+  const pool = new Pool();
+  const toBranch = await getBranchNameOfRef(toRef, ELECTRON_DIR);
+
+  console.log(`Generating release notes between ${fromRef} and ${toRef} for version ${newVersion} in branch ${toBranch}`);
 
   // get the electron/electron commits
-  const electron = { owner: 'electron', repo: 'electron', dir: ELECTRON_VERSION };
+  const electron = { owner: 'electron', repo: 'electron', dir: ELECTRON_DIR };
   await addRepoToPool(pool, electron, fromRef, toRef);
 
-  // Don't include submodules if comparing across major versions;
-  // there's just too much churn otherwise.
-  const includeDeps = semver.valid(fromRef) &&
-                      semver.valid(toRef) &&
-                      semver.major(fromRef) === semver.major(toRef);
-
-  if (includeDeps) {
-    await getDependencyCommitsGN(pool, fromRef, toRef);
-  }
-
   // remove any old commits
   pool.commits = pool.commits.filter(commit => !pool.processedHashes.has(commit.hash));
 
@@ -449,89 +424,27 @@ const getNotes = async (fromRef, toRef, newVersion) => {
     pool.processedHashes.add(revertHash);
   }
 
-  // scrape PRs for release note 'Notes:' comments
+  // ensure the commit has a note
   for (const commit of pool.commits) {
-    let pr = commit.pr;
-
-    let prSubject;
-    while (pr && !commit.note) {
-      const note = await getNoteFromClerk(pr.number, pr.owner, pr.repo);
-      if (note) {
-        commit.note = note;
-      }
-
-      // if we already have all the data we need, stop scraping the PRs
-      if (commit.note && commit.type && prSubject) {
+    for (const prKey of commit.prKeys.values()) {
+      if (commit.note) {
         break;
       }
-
-      const prData = await getPullRequest(pr.number, pr.owner, pr.repo);
-      if (!prData || !prData.data) {
-        break;
-      }
-
-      // try to pull a release note from the pull comment
-      const prParsed = parseCommitMessage(`${prData.data.title}\n\n${prData.data.body}`, pr.owner, pr.repo);
-      if (!commit.note) {
-        commit.note = prParsed.note;
-      }
-      if (!commit.type || prParsed.type === 'breaking-change') {
-        commit.type = prParsed.type;
-      }
-      prSubject = prSubject || prParsed.subject;
-
-      pr = prParsed.pr && (prParsed.pr.number !== pr.number) ? prParsed.pr : null;
+      commit.note = await getNoteFromClerk(prKey);
     }
-
-    // if we still don't have a note, it's because someone missed a 'Notes:
-    // comment in a PR somewhere... use the PR subject as a fallback.
-    commit.note = commit.note || prSubject;
   }
 
   // remove non-user-facing commits
   pool.commits = pool.commits
+    .filter(commit => commit && commit.note)
     .filter(commit => commit.note !== NO_NOTES)
-    .filter(commit => !((commit.note || commit.subject).match(/^[Bb]ump v\d+\.\d+\.\d+/)));
-
-  if (!shouldIncludeMultibranchChanges(newVersion)) {
-    // load all the prDatas
-    await Promise.all(
-      pool.commits.map(commit => new Promise(async (resolve) => {
-        const { pr } = commit;
-        if (typeof pr === 'object') {
-          const prData = await getPullRequest(pr.number, pr.owner, pr.repo);
-          if (prData) {
-            commit.prData = prData;
-          }
-        }
-        resolve();
-      }))
-    );
+    .filter(commit => commit.note.match(/^[Bb]ump v\d+\.\d+\.\d+/) === null);
 
-    // remove items that already landed in a previous major/minor series
-    pool.commits = pool.commits
-      .filter(commit => {
-        if (!commit.prData) {
-          return true;
-        }
-        const reducer = (accumulator, current) => {
-          if (!semver.valid(accumulator)) { return current; }
-          if (!semver.valid(current)) { return accumulator; }
-          return semver.lt(accumulator, current) ? accumulator : current;
-        };
-        const earliestRelease = commit.prData.data.labels
-          .map(label => label.name.match(/merged\/(\d+)-(\d+)-x/))
-          .filter(label => !!label)
-          .map(label => `${label[1]}.${label[2]}.0`)
-          .reduce(reducer, null);
-        if (!semver.valid(earliestRelease)) {
-          return true;
-        }
-        return semver.diff(earliestRelease, newVersion).includes('patch');
-      });
+  for (const commit of pool.commits) {
+    commit.trops = await getMergedTrops(commit, pool);
   }
 
-  pool.commits = removeSupercededChromiumUpdates(pool.commits);
+  pool.commits = removeSupercededStackUpdates(pool.commits);
 
   const notes = {
     breaking: [],
@@ -540,15 +453,16 @@ const getNotes = async (fromRef, toRef, newVersion) => {
     fix: [],
     other: [],
     unknown: [],
-    name: newVersion
+    name: newVersion,
+    toBranch
   };
 
   pool.commits.forEach(commit => {
-    const str = commit.type;
-    if (!str) {
-      notes.unknown.push(commit);
-    } else if (breakTypes.has(str)) {
+    const str = commit.semanticType;
+    if (commit.isBreakingChange) {
       notes.breaking.push(commit);
+    } else if (!str) {
+      notes.unknown.push(commit);
     } else if (docTypes.has(str)) {
       notes.docs.push(commit);
     } else if (featTypes.has(str)) {
@@ -565,49 +479,76 @@ const getNotes = async (fromRef, toRef, newVersion) => {
   return notes;
 };
 
-const removeSupercededChromiumUpdates = (commits) => {
-  const chromiumRegex = /^Updated Chromium to \d+\.\d+\.\d+\.\d+/;
-  const updates = commits.filter(commit => (commit.note || commit.subject).match(chromiumRegex));
-  const keepers = commits.filter(commit => !updates.includes(commit));
+const removeSupercededStackUpdates = (commits) => {
+  const updateRegex = /^Updated ([a-zA-Z.]+) to v?([\d.]+)/;
+  const notupdates = [];
 
-  // keep the newest update.
-  if (updates.length) {
-    updates.sort((a, b) => a.originalPr.number - b.originalPr.number);
-    keepers.push(updates.pop());
+  const newest = {};
+  for (const commit of commits) {
+    const match = (commit.note || commit.subject).match(updateRegex);
+    if (!match) {
+      notupdates.push(commit);
+      continue;
+    }
+    const [, dep, version] = match;
+    if (!newest[dep] || newest[dep].version < version) {
+      newest[dep] = { commit, version };
+    }
   }
 
-  return keepers;
+  return [...notupdates, ...Object.values(newest).map(o => o.commit)];
 };
 
 /***
 ****  Render
 ***/
 
-const renderLink = (commit, explicitLinks) => {
-  let link;
-  const pr = commit.originalPr;
-  if (pr) {
-    const { owner, repo, number } = pr;
-    const url = `https://github.com/${owner}/${repo}/pull/${number}`;
-    const text = owner === 'electron' && repo === 'electron'
-      ? `#${number}`
-      : `${owner}/${repo}#${number}`;
-    link = explicitLinks ? `[${text}](${url})` : text;
-  } else {
-    const { owner, repo, hash } = commit;
-    const url = `https://github.com/${owner}/${repo}/commit/${hash}`;
-    const text = owner === 'electron' && repo === 'electron'
-      ? `${hash.slice(0, 8)}`
-      : `${owner}/${repo}@${hash.slice(0, 8)}`;
-    link = explicitLinks ? `[${text}](${url})` : text;
-  }
-  return link;
-};
+// @return the pull request's GitHub URL
+const buildPullURL = ghKey => `https://github.com/${ghKey.owner}/${ghKey.repo}/pull/${ghKey.number}`;
+
+const renderPull = ghKey => `[#${ghKey.number}](${buildPullURL(ghKey)})`;
+
+// @return the commit's GitHub URL
+const buildCommitURL = commit => `https://github.com/${commit.owner}/${commit.repo}/commit/${commit.hash}`;
+
+const renderCommit = commit => `[${commit.hash.slice(0, 8)}](${buildCommitURL(commit)})`;
+
+// @return a markdown link to the PR if available; otherwise, the git commit
+function renderLink (commit) {
+  const maybePull = commit.prKeys.values().next();
+  return maybePull.value ? renderPull(maybePull.value) : renderCommit(commit);
+}
+
+// @return a terser branch name,
+//   e.g. '7-2-x' -> '7.2' and '8-x-y' -> '8'
+const renderBranchName = name => name.replace(/-[a-zA-Z]/g, '').replace('-', '.');
+
+const renderTrop = (branch, ghKey) => `[${renderBranchName(branch)}](${buildPullURL(ghKey)})`;
+
+// @return markdown-formatted links to other branches' trops,
+//   e.g. "(Also in 7.2, 8, 9)"
+function renderTrops (commit, excludeBranch) {
+  const body = [...commit.trops.entries()]
+    .filter(([branch]) => branch !== excludeBranch)
+    .sort(([branchA], [branchB]) => parseInt(branchA) - parseInt(branchB)) // sort by semver major
+    .map(([branch, key]) => renderTrop(branch, key))
+    .join(', ');
+  return body ? `<span style="font-size:small;">(Also in ${body})</span>` : body;
+}
 
-const renderCommit = (commit, explicitLinks) => {
-  // clean up the note
-  let note = commit.note || commit.subject;
+// @return a slightly cleaned-up human-readable change description
+function renderDescription (commit) {
+  let note = commit.note || commit.subject || '';
   note = note.trim();
+
+  // release notes bullet point every change, so if the note author
+  // manually started the content with a bullet point, that will confuse
+  // the markdown renderer -- remove the redundant bullet point
+  // (example: https://github.com/electron/electron/pull/25216)
+  if (note.startsWith('*')) {
+    note = note.slice(1).trim();
+  }
+
   if (note.length !== 0) {
     note = note[0].toUpperCase() + note.substr(1);
 
@@ -616,22 +557,22 @@ const renderCommit = (commit, explicitLinks) => {
     }
 
     const commonVerbs = {
-      'Added': [ 'Add' ],
-      'Backported': [ 'Backport' ],
-      'Cleaned': [ 'Clean' ],
-      'Disabled': [ 'Disable' ],
-      'Ensured': [ 'Ensure' ],
-      'Exported': [ 'Export' ],
-      'Fixed': [ 'Fix', 'Fixes' ],
-      'Handled': [ 'Handle' ],
-      'Improved': [ 'Improve' ],
-      'Made': [ 'Make' ],
-      'Removed': [ 'Remove' ],
-      'Repaired': [ 'Repair' ],
-      'Reverted': [ 'Revert' ],
-      'Stopped': [ 'Stop' ],
-      'Updated': [ 'Update' ],
-      'Upgraded': [ 'Upgrade' ]
+      Added: ['Add'],
+      Backported: ['Backport'],
+      Cleaned: ['Clean'],
+      Disabled: ['Disable'],
+      Ensured: ['Ensure'],
+      Exported: ['Export'],
+      Fixed: ['Fix', 'Fixes'],
+      Handled: ['Handle'],
+      Improved: ['Improve'],
+      Made: ['Make'],
+      Removed: ['Remove'],
+      Repaired: ['Repair'],
+      Reverted: ['Revert'],
+      Stopped: ['Stop'],
+      Updated: ['Update'],
+      Upgraded: ['Upgrade']
     };
     for (const [key, values] of Object.entries(commonVerbs)) {
       for (const value of values) {
@@ -643,30 +584,24 @@ const renderCommit = (commit, explicitLinks) => {
     }
   }
 
-  const link = renderLink(commit, explicitLinks);
+  return note;
+}
 
-  return { note, link };
-};
+// @return markdown-formatted release note line item,
+//   e.g. '* Fixed a foo. #12345 (Also in 7.2, 8, 9)'
+const renderNote = (commit, excludeBranch) =>
+  `* ${renderDescription(commit)} ${renderLink(commit)} ${renderTrops(commit, excludeBranch)}\n`;
 
-const renderNotes = (notes, explicitLinks) => {
-  const rendered = [ `# Release Notes for ${notes.name}\n\n` ];
+const renderNotes = (notes) => {
+  const rendered = [`# Release Notes for ${notes.name}\n\n`];
 
   const renderSection = (title, commits) => {
-    if (commits.length === 0) {
-      return;
-    }
-    const notes = new Map();
-    for (const note of commits.map(commit => renderCommit(commit, explicitLinks))) {
-      if (!notes.has(note.note)) {
-        notes.set(note.note, [note.link]);
-      } else {
-        notes.get(note.note).push(note.link);
-      }
+    if (commits.length > 0) {
+      rendered.push(
+        `## ${title}\n\n`,
+        ...(commits.map(commit => renderNote(commit, notes.toBranch)).sort())
+      );
     }
-    rendered.push(`## ${title}\n\n`);
-    const lines = [];
-    notes.forEach((links, key) => lines.push(` * ${key} ${links.map(link => link.toString()).sort().join(', ')}\n`));
-    rendered.push(...lines.sort(), '\n');
   };
 
   renderSection('Breaking Changes', notes.breaking);
@@ -675,7 +610,7 @@ const renderNotes = (notes, explicitLinks) => {
   renderSection('Other Changes', notes.other);
 
   if (notes.docs.length) {
-    const docs = notes.docs.map(commit => renderLink(commit, explicitLinks)).sort();
+    const docs = notes.docs.map(commit => renderLink(commit)).sort();
     rendered.push('## Documentation\n\n', ` * Documentation changes: ${docs.join(', ')}\n`, '\n');
   }
 

+ 5 - 1
script/release/prepare-release.js

@@ -211,4 +211,8 @@ async function prepareRelease (isBeta, notesOnly) {
   }
 }
 
-prepareRelease(!args.stable, args.notesOnly);
+prepareRelease(!args.stable, args.notesOnly)
+  .catch((err) => {
+    console.error(err);
+    process.exit(1);
+  });

File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-0600420bac25439fc2067d51c6aaa4ee11770577


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-2955c67c4ea712fa22773ac9113709fc952bfd49


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-2fad53e66b1a2cb6f7dad88fe9bb62d7a461fe98


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-467409458e716c68b35fa935d556050ca6bed1c4


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-61dc1c88fd34a3e8fff80c80ed79d0455970e610


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-89eb309d0b22bd4aec058ffaf983e81e56a5c378


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-8bc0c92137f4a77dc831ca644a86a3e48b51a11e


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-commit-a6ff42c190cb5caf8f3e217748e49183a951491b


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-20214-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-21497-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-21891-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-21946-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-22750-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-22770-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-22828-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-25052-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-issue-25216-comments


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-20214


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-20620


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-21497


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-21591


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-21891


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-21946


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-22750


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-22770


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-22828


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-25052


File diff suppressed because it is too large
+ 0 - 0
spec-main/fixtures/release-notes/cache/electron-electron-pull-25216


+ 2 - 0
spec-main/package.json

@@ -4,9 +4,11 @@
   "main": "index.js",
   "version": "0.1.0",
   "devDependencies": {
+    "@types/sinon": "^9.0.4",
     "@types/ws": "^7.2.0",
     "echo": "file:fixtures/native-addon/echo",
     "q": "^1.5.1",
+    "sinon": "^9.0.1",
     "ws": "^7.2.1"
   }
 }

+ 213 - 0
spec-main/release-notes-spec.ts

@@ -0,0 +1,213 @@
+import { GitProcess, IGitExecutionOptions, IGitResult } from 'dugite';
+import { expect } from 'chai';
+import * as notes from '../script/release/notes/notes.js';
+import * as path from 'path';
+import * as sinon from 'sinon';
+
+/* Fake a Dugite GitProcess that only returns the specific
+   commits that we want to test */
+
+class Commit {
+  sha1: string;
+  subject: string;
+  constructor (sha1: string, subject: string) {
+    this.sha1 = sha1;
+    this.subject = subject;
+  }
+}
+
+class GitFake {
+  branches: {
+    [key: string]: Commit[],
+  };
+
+  constructor () {
+    this.branches = {};
+  }
+
+  setBranch (name: string, commits: Array<Commit>): void {
+    this.branches[name] = commits;
+  }
+
+  // find the newest shared commit between branches a and b
+  mergeBase (a: string, b:string): string {
+    for (const commit of [...this.branches[a].reverse()]) {
+      if (this.branches[b].map((commit: Commit) => commit.sha1).includes(commit.sha1)) {
+        return commit.sha1;
+      }
+    }
+    console.error('test error: branches not related');
+    return '';
+  }
+
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  exec (args: string[], path: string, options?: IGitExecutionOptions | undefined): Promise<IGitResult> {
+    let stdout = '';
+    const stderr = '';
+    const exitCode = 0;
+
+    if (args.length === 3 && args[0] === 'merge-base') {
+      // expected form: `git merge-base branchName1 branchName2`
+      const a: string = args[1]!;
+      const b: string = args[2]!;
+      stdout = this.mergeBase(a, b);
+    } else if (args.length === 3 && args[0] === 'log' && args[1] === '--format=%H') {
+      // exepcted form: `git log --format=%H branchName
+      const branch: string = args[2]!;
+      stdout = this.branches[branch].map((commit: Commit) => commit.sha1).join('\n');
+    } else if (args.length > 1 && args[0] === 'log' && args.includes('--format=%H,%s')) {
+      // expected form: `git log --format=%H,%s sha1..branchName
+      const [start, branch] = args[args.length - 1].split('..');
+      const lines : string[] = [];
+      let started = false;
+      for (const commit of this.branches[branch]) {
+        started = started || commit.sha1 === start;
+        if (started) {
+          lines.push(`${commit.sha1},${commit.subject}` /* %H,%s */);
+        }
+      }
+      stdout = lines.join('\n');
+    } else if (args.length === 6 &&
+               args[0] === 'branch' &&
+               args[1] === '--all' &&
+               args[2] === '--contains' &&
+               args[3].endsWith('-x-y')) {
+      // "what branch is this tag in?"
+      // git branch --all --contains ${ref} --sort version:refname
+      stdout = args[3];
+    } else {
+      console.error('unhandled GitProcess.exec():', args);
+    }
+
+    return Promise.resolve({ exitCode, stdout, stderr });
+  }
+}
+
+describe('release notes', () => {
+  const sandbox = sinon.createSandbox();
+  const gitFake = new GitFake();
+
+  const oldBranch = '8-x-y';
+  const newBranch = '9-x-y';
+
+  // commits shared by both oldBranch and newBranch
+  const sharedHistory = [
+    new Commit('2abea22b4bffa1626a521711bacec7cd51425818', "fix: explicitly cancel redirects when mode is 'error' (#20686)"),
+    new Commit('467409458e716c68b35fa935d556050ca6bed1c4', 'build: add support for automated minor releases (#20620)') // merge-base
+  ];
+
+  // these commits came after newBranch was created
+  const newBreaking = new Commit('2fad53e66b1a2cb6f7dad88fe9bb62d7a461fe98', 'refactor: use v8 serialization for ipc (#20214)');
+  const newFeat = new Commit('89eb309d0b22bd4aec058ffaf983e81e56a5c378', 'feat: allow GUID parameter to avoid systray demotion on Windows  (#21891)');
+  const newFix = new Commit('0600420bac25439fc2067d51c6aaa4ee11770577', "fix: don't allow window to go behind menu bar on mac (#22828)");
+  const oldFix = new Commit('f77bd19a70ac2d708d17ddbe4dc12745ca3a8577', 'fix: prevent menu gc during popup (#20785)');
+
+  // a bug that's fixed in both branches by separate PRs
+  const newTropFix = new Commit('a6ff42c190cb5caf8f3e217748e49183a951491b', 'fix: workaround for hang when preventDefault-ing nativeWindowOpen (#22750)');
+  const oldTropFix = new Commit('8751f485c5a6c8c78990bfd55a4350700f81f8cd', 'fix: workaround for hang when preventDefault-ing nativeWindowOpen (#22749)');
+
+  // a PR that has unusual note formatting
+  const sublist = new Commit('61dc1c88fd34a3e8fff80c80ed79d0455970e610', 'fix: client area inset calculation when maximized for framless windows (#25052) (#25216)');
+
+  before(() => {
+    // location of relase-notes' octokit reply cache
+    const fixtureDir = path.resolve(__dirname, 'fixtures', 'release-notes');
+    process.env.NOTES_CACHE_PATH = path.resolve(fixtureDir, 'cache');
+  });
+
+  beforeEach(() => {
+    const wrapper = (args: string[], path: string, options?: IGitExecutionOptions | undefined) => gitFake.exec(args, path, options);
+    sandbox.replace(GitProcess, 'exec', wrapper);
+
+    gitFake.setBranch(oldBranch, [...sharedHistory, oldFix]);
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  describe('trop annotations', () => {
+    it('shows sibling branches', async function () {
+      const version = 'v9.0.0';
+      gitFake.setBranch(oldBranch, [...sharedHistory, oldTropFix]);
+      gitFake.setBranch(newBranch, [...sharedHistory, newTropFix]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      expect(results.fix).to.have.lengthOf(1);
+      expect(results.fix[0].trops).to.have.keys('8-x-y', '9-x-y');
+    });
+  });
+
+  // use case: A malicious contributor could edit the text of their 'Notes:'
+  // in the PR body after a PR's been merged and the maintainers have moved on.
+  // So instead always use the release-clerk PR comment
+  it('uses the release-clerk text', async function () {
+    // realText source: ${fixtureDir}/electron-electron-issue-21891-comments
+    const realText = 'Added GUID parameter to Tray API to avoid system tray icon demotion on Windows';
+    const testCommit = new Commit('89eb309d0b22bd4aec058ffaf983e81e56a5c378', 'feat: lole u got troled hard (#21891)');
+    const version = 'v9.0.0';
+
+    gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+    const results: any = await notes.get(oldBranch, newBranch, version);
+    expect(results.feat).to.have.lengthOf(1);
+    expect(results.feat[0].hash).to.equal(testCommit.sha1);
+    expect(results.feat[0].note).to.equal(realText);
+  });
+
+  describe('rendering', () => {
+    it('removes redundant bullet points', async function () {
+      const testCommit = sublist;
+      const version = 'v10.1.1';
+
+      gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      const rendered: any = await notes.render(results);
+
+      expect(rendered).to.not.include('* *');
+    });
+
+    it('indents sublists', async function () {
+      const testCommit = sublist;
+      const version = 'v10.1.1';
+
+      gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      const rendered: any = await notes.render(results);
+
+      expect(rendered).to.include([
+        '* Fixed the following issues for frameless when maximized on Windows:',
+        '  * fix unreachable task bar when auto hidden with position top',
+        '  * fix 1px extending to secondary monitor',
+        '  * fix 1px overflowing into taskbar at certain resolutions',
+        '  * fix white line on top of window under 4k resolutions. [#25216]'].join('\n'));
+    });
+  });
+  // test that when you feed in different semantic commit types,
+  // the parser returns them in the results' correct category
+  describe('semantic commit', () => {
+    const version = 'v9.0.0';
+
+    it("honors 'feat' type", async function () {
+      const testCommit = newFeat;
+      gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      expect(results.feat).to.have.lengthOf(1);
+      expect(results.feat[0].hash).to.equal(testCommit.sha1);
+    });
+
+    it("honors 'fix' type", async function () {
+      const testCommit = newFix;
+      gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      expect(results.fix).to.have.lengthOf(1);
+      expect(results.fix[0].hash).to.equal(testCommit.sha1);
+    });
+
+    it("honors 'BREAKING CHANGE' message", async function () {
+      const testCommit = newBreaking;
+      gitFake.setBranch(newBranch, [...sharedHistory, testCommit]);
+      const results: any = await notes.get(oldBranch, newBranch, version);
+      expect(results.breaking).to.have.lengthOf(1);
+      expect(results.breaking[0].hash).to.equal(testCommit.sha1);
+    });
+  });
+});

+ 116 - 0
spec-main/yarn.lock

@@ -2,11 +2,59 @@
 # yarn lockfile v1
 
 
+"@sinonjs/commons@^1", "@sinonjs/commons@^1.6.0", "@sinonjs/commons@^1.7.0", "@sinonjs/commons@^1.7.2":
+  version "1.8.1"
+  resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.8.1.tgz#e7df00f98a203324f6dc7cc606cad9d4a8ab2217"
+  integrity sha512-892K+kWUUi3cl+LlqEWIDrhvLgdL79tECi8JZUyq6IviKy/DNhuzCRlbHUjxK89f4ypPMMaFnFuR9Ie6DoIMsw==
+  dependencies:
+    type-detect "4.0.8"
+
+"@sinonjs/fake-timers@^6.0.0", "@sinonjs/fake-timers@^6.0.1":
+  version "6.0.1"
+  resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz#293674fccb3262ac782c7aadfdeca86b10c75c40"
+  integrity sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA==
+  dependencies:
+    "@sinonjs/commons" "^1.7.0"
+
+"@sinonjs/formatio@^5.0.1":
+  version "5.0.1"
+  resolved "https://registry.yarnpkg.com/@sinonjs/formatio/-/formatio-5.0.1.tgz#f13e713cb3313b1ab965901b01b0828ea6b77089"
+  integrity sha512-KaiQ5pBf1MpS09MuA0kp6KBQt2JUOQycqVG1NZXvzeaXe5LGFqAKueIS0bw4w0P9r7KuBSVdUk5QjXsUdu2CxQ==
+  dependencies:
+    "@sinonjs/commons" "^1"
+    "@sinonjs/samsam" "^5.0.2"
+
+"@sinonjs/samsam@^5.0.2", "@sinonjs/samsam@^5.1.0":
+  version "5.1.0"
+  resolved "https://registry.yarnpkg.com/@sinonjs/samsam/-/samsam-5.1.0.tgz#3afe719232b541bb6cf3411a4c399a188de21ec0"
+  integrity sha512-42nyaQOVunX5Pm6GRJobmzbS7iLI+fhERITnETXzzwDZh+TtDr/Au3yAvXVjFmZ4wEUaE4Y3NFZfKv0bV0cbtg==
+  dependencies:
+    "@sinonjs/commons" "^1.6.0"
+    lodash.get "^4.4.2"
+    type-detect "^4.0.8"
+
+"@sinonjs/text-encoding@^0.7.1":
+  version "0.7.1"
+  resolved "https://registry.yarnpkg.com/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz#8da5c6530915653f3a1f38fd5f101d8c3f8079c5"
+  integrity sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==
+
 "@types/node@*":
   version "13.7.0"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-13.7.0.tgz#b417deda18cf8400f278733499ad5547ed1abec4"
   integrity sha512-GnZbirvmqZUzMgkFn70c74OQpTTUcCzlhQliTzYjQMqg+hVKcDnxdL19Ne3UdYzdMA/+W3eb646FWn/ZaT1NfQ==
 
+"@types/sinon@^9.0.4":
+  version "9.0.5"
+  resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-9.0.5.tgz#56b2a12662dd8c7d081cdc511af5f872cb37377f"
+  integrity sha512-4CnkGdM/5/FXDGqL32JQ1ttVrGvhOoesLLF7VnTh4KdjK5N5VQOtxaylFqqTjnHx55MnD9O02Nbk5c1ELC8wlQ==
+  dependencies:
+    "@types/sinonjs__fake-timers" "*"
+
+"@types/sinonjs__fake-timers@*":
+  version "6.0.1"
+  resolved "https://registry.yarnpkg.com/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-6.0.1.tgz#681df970358c82836b42f989188d133e218c458e"
+  integrity sha512-yYezQwGWty8ziyYLdZjwxyMb0CZR49h8JALHGrxjQHWlqGgc8kLdHEgWrgL0uZ29DMvEVBDnHU2Wg36zKSIUtA==
+
 "@types/ws@^7.2.0":
   version "7.2.1"
   resolved "https://registry.yarnpkg.com/@types/ws/-/ws-7.2.1.tgz#b800f2b8aee694e2b581113643e20d79dd3b8556"
@@ -14,14 +62,82 @@
   dependencies:
     "@types/node" "*"
 
+diff@^4.0.2:
+  version "4.0.2"
+  resolved "https://registry.yarnpkg.com/diff/-/diff-4.0.2.tgz#60f3aecb89d5fae520c11aa19efc2bb982aade7d"
+  integrity sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==
+
 "echo@file:fixtures/native-addon/echo":
   version "0.0.1"
 
+has-flag@^4.0.0:
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/has-flag/-/has-flag-4.0.0.tgz#944771fd9c81c81265c4d6941860da06bb59479b"
+  integrity sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==
+
[email protected]:
+  version "0.0.1"
+  resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf"
+  integrity sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=
+
+just-extend@^4.0.2:
+  version "4.1.0"
+  resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-4.1.0.tgz#7278a4027d889601640ee0ce0e5a00b992467da4"
+  integrity sha512-ApcjaOdVTJ7y4r08xI5wIqpvwS48Q0PBG4DJROcEkH1f8MdAiNFyFxz3xoL0LWAVwjrwPYZdVHHxhRHcx/uGLA==
+
+lodash.get@^4.4.2:
+  version "4.4.2"
+  resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99"
+  integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=
+
+nise@^4.0.4:
+  version "4.0.4"
+  resolved "https://registry.yarnpkg.com/nise/-/nise-4.0.4.tgz#d73dea3e5731e6561992b8f570be9e363c4512dd"
+  integrity sha512-bTTRUNlemx6deJa+ZyoCUTRvH3liK5+N6VQZ4NIw90AgDXY6iPnsqplNFf6STcj+ePk0H/xqxnP75Lr0J0Fq3A==
+  dependencies:
+    "@sinonjs/commons" "^1.7.0"
+    "@sinonjs/fake-timers" "^6.0.0"
+    "@sinonjs/text-encoding" "^0.7.1"
+    just-extend "^4.0.2"
+    path-to-regexp "^1.7.0"
+
+path-to-regexp@^1.7.0:
+  version "1.8.0"
+  resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.8.0.tgz#887b3ba9d84393e87a0a0b9f4cb756198b53548a"
+  integrity sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==
+  dependencies:
+    isarray "0.0.1"
+
 q@^1.5.1:
   version "1.5.1"
   resolved "https://registry.yarnpkg.com/q/-/q-1.5.1.tgz#7e32f75b41381291d04611f1bf14109ac00651d7"
   integrity sha1-fjL3W0E4EpHQRhHxvxQQmsAGUdc=
 
+sinon@^9.0.1:
+  version "9.0.3"
+  resolved "https://registry.yarnpkg.com/sinon/-/sinon-9.0.3.tgz#bffc3ec28c936332cd2a41833547b9eed201ecff"
+  integrity sha512-IKo9MIM111+smz9JGwLmw5U1075n1YXeAq8YeSFlndCLhAL5KGn6bLgu7b/4AYHTV/LcEMcRm2wU2YiL55/6Pg==
+  dependencies:
+    "@sinonjs/commons" "^1.7.2"
+    "@sinonjs/fake-timers" "^6.0.1"
+    "@sinonjs/formatio" "^5.0.1"
+    "@sinonjs/samsam" "^5.1.0"
+    diff "^4.0.2"
+    nise "^4.0.4"
+    supports-color "^7.1.0"
+
+supports-color@^7.1.0:
+  version "7.2.0"
+  resolved "https://registry.yarnpkg.com/supports-color/-/supports-color-7.2.0.tgz#1b7dcdcb32b8138801b3e478ba6a51caa89648da"
+  integrity sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==
+  dependencies:
+    has-flag "^4.0.0"
+
[email protected], type-detect@^4.0.8:
+  version "4.0.8"
+  resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.8.tgz#7646fb5f18871cfbb7749e69bd39a6388eb7450c"
+  integrity sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==
+
 ws@^7.2.1:
   version "7.2.1"
   resolved "https://registry.yarnpkg.com/ws/-/ws-7.2.1.tgz#03ed52423cd744084b2cf42ed197c8b65a936b8e"

Some files were not shown because too many files changed in this diff