Browse Source

chore: graceful handling of notes with sub-lists (#25312)

* chore: graceful handling of notes with sub-lists

Handle multine release notes that contain their own bullet points.

Also, if a release note begins with a bullet point, remove it because it
will confuse the markdown parser to have two bullet points.

* chore: make lint happy

* test: add tests for release note changes

* chore: only target current octokit

* chore: add commits to release-notes test cache

No behavior change; just includes files that ought to be cached to prevent
hitting octokit for them.

Co-authored-by: Charles Kerr <[email protected]>
trop[bot] 4 years ago
parent
commit
7745e48498

+ 21 - 3
script/release/notes/notes.js

@@ -6,6 +6,7 @@ const fs = require('fs');
 const path = require('path');
 
 const { GitProcess } = require('dugite');
+
 const { Octokit } = require('@octokit/rest');
 const octokit = new Octokit({
   auth: process.env.ELECTRON_GITHUB_TOKEN
@@ -109,13 +110,21 @@ const getNoteFromClerk = async (ghKey) => {
       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/) // 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();
     }
   }
@@ -532,6 +541,15 @@ function renderTrops (commit, excludeBranch) {
 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);
 

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-issue-21497-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-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-25052


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


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

@@ -106,6 +106,9 @@ describe('release notes', () => {
   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');
@@ -151,6 +154,34 @@ describe('release notes', () => {
     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', () => {

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