New --cc option to copy people in issues (#732)

The CLI now accepts a `--cc` option that can take a list of GitHub
handles. It will add a Cc message with these handles at the end of
issues when these issues may warrant more discussion (only the case
for algorithms and Web IDL anomalies for now).

The option is only meaningful if the --issues option is set.

To avoid hardcoding names in code, a new `CC` variable was added to
the repository, initially set to "dontcallmedom tidoust", so as to end up with:

```
<sub>Cc @dontcallmedom @tidoust</sub>
```

.github/workflows/file-issue-for-review.yml

@@ -30,7 +30,16 @@ jobs:
       working-directory: strudy
     - name: Run Strudy to detect new anomalies
       working-directory: strudy
-      run: node strudy.js inspect ../webref --issues issues --what brokenLinks discontinuedReferences incompatiblePartialIdlExposure unexpectedEventHandler wrongCaseEnumValue --update-mode old
+      run: |
+        node strudy.js inspect ../webref \
+          --issues issues \
+          --update-mode old \
+          --cc ${{ vars.CC }} \
+          --what brokenLinks \
+          --what discontinuedReferences \
+          --what incompatiblePartialIdlExposure \
+          --what unexpectedEventHandler \
+          --what wrongCaseEnumValue
     - name: Run issue filer script
       working-directory: strudy
       run: node src/reporting/file-issue-for-review.js --max 10

src/lib/study.js

@@ -109,7 +109,8 @@ const anomalyGroups = [
         guidance: 'See [Dealing with the event loop](https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-for-spec-authors) in the HTML specification for guidance on how to deal with algorithm sections that run *in parallel*.'
       }
     ],
-    study: studyAlgorithms
+    study: studyAlgorithms,
+    cc: true
   },
 
   {
@@ -178,7 +179,8 @@ const anomalyGroups = [
       { name: 'wrongType', title: 'Web IDL names incorrectly used as types' }
     ],
     study: studyWebIdl,
-    studyParams: ['curated']
+    studyParams: ['curated'],
+    cc: true
   }
 ];
 
@@ -406,7 +408,7 @@ function pad(str, depth) {
   return str;
 }
 
-function serializeEntry(entry, format, depth = 0) {
+function serializeEntry(entry, format, options, depth = 0) {
   let res;
   if (format === 'json') {
     res = Object.assign({}, entry);
@@ -425,10 +427,10 @@ function serializeEntry(entry, format, depth = 0) {
       }));
     }
     if (entry.items) {
-      res.items = entry.items.map(item => serializeEntry(item, format, depth + 1));
+      res.items = entry.items.map(item => serializeEntry(item, format, options, depth + 1));
     }
     if (entry.anomalies) {
-      res.anomalies = entry.anomalies.map(anomaly => serializeEntry(anomaly, format, depth + 1));
+      res.anomalies = entry.anomalies.map(anomaly => serializeEntry(anomaly, format, options, depth + 1));
     }
   }
   else if (format === 'markdown') {
@@ -481,18 +483,33 @@ function serializeEntry(entry, format, depth = 0) {
     }
 
     for (const item of entry.items ?? []) {
-      res += '\n' + serializeEntry(item, format, depth + 1);
+      res += '\n' + serializeEntry(item, format, options, depth + 1);
     }
     if (entry.anomalies?.length > 0) {
       for (const anomaly of entry.anomalies) {
-        res += `\n` + serializeEntry(anomaly, format, depth + 1);
+        res += `\n` + serializeEntry(anomaly, format, options, depth + 1);
       }
       if (entry.group?.guidance) {
         res += `\n\n` + entry.group.guidance;
       }
       else if (entry.type?.guidance) {
         res += `\n\n` + entry.type.guidance;
       }
+      if (options?.cc?.length > 0 && entry.type) {
+        const mentions = options.cc
+          .map(name => name.startsWith('@') ? name : '@' + name)
+          .join(' ')
+        if (entry.type.cc) {
+          res += `\n\n<sub>Cc ${mentions}</sub>`;
+        }
+        else {
+          const group = anomalyGroups.find(group =>
+            group.types.find(type => type.name === entry.type.name));
+          if (group.cc) {
+            res += `\n\n<sub>Cc ${mentions}</sub>`;
+          }
+        }
+      }
     }
   }
   return res;
@@ -502,19 +519,19 @@ function serializeEntry(entry, format, depth = 0) {
 /**
  * Format the structured report as JSON or markdown, or a combination of both
  */
-function formatReport(format, report) {
+function formatReport(format, report, options) {
   if (format === 'json') {
     // We'll return the report as is, trimming the information about specs to
     // a reasonable minimum (the rest of the information can easily be
     // retrieved from the crawl result if needed)
-    return report.map(entry => serializeEntry(entry, 'json'));
+    return report.map(entry => serializeEntry(entry, 'json', options));
   }
   else if (format === 'issue') {
     return report.map(entry => Object.assign({
       name: entry.name,
       title: entry.title,
       spec: entry.spec,
-      content: serializeEntry(entry, 'markdown')
+      content: serializeEntry(entry, 'markdown', options)
     }));
   }
   else if (format === 'full') {
@@ -524,11 +541,11 @@ function formatReport(format, report) {
         content: report.map(entry => {
           if (entry.title) {
             return `## ${entry.title}
-${serializeEntry(entry, 'markdown')}
+${serializeEntry(entry, 'markdown', options)}
 `;
           }
           else {
-            return serializeEntry(entry, 'markdown');
+            return serializeEntry(entry, 'markdown', options);
           }
         })
       }
@@ -673,7 +690,7 @@ export default async function study(specs, options = {}) {
       studied: specs.length,
       anomalies: anomalies.length
     },
-    results: formatReport(format, report),
+    results: formatReport(format, report, options),
     looksGood: getNamesOfNonReportedEntries(report, specs, what, structure)
   };
 

strudy.js

@@ -64,6 +64,7 @@ program
   .command('inspect')
   .alias('study')
   .argument('<crawl>', 'Path/URL to crawl report')
+  .option('-cc, --cc <names...>', 'people to Cc in issues that may need help')
   .option('-f, --format <format>', 'report markdown or json', 'markdown')
   .option('-i, --issues <folder>', 'report issues as markdown files in the given folder')
   .option('-m, --max <max>', 'maximum number of issue files to create/update', myParseInt, 0)
@@ -95,6 +96,22 @@ Argument:
   then for an "index.json" file.
 
 Usage notes for some of the options:
+-cc, --cc <names...>
+  Lists people to copy in issues with a "Cc" message so that they get notified.
+  This is helpful to follow issues that may warrant further discussion and
+  guidance.
+
+  Each name should be a GitHub handle, such as "tidoust" or "dontcallmedom".
+  The handle may start with a "@" (code will add it as prefix automatically
+  otherwise).
+
+  The "Cc" message will only be added to anomalies that are not obvious to fix:
+  for example, it will be set for anomalies about algorithms and Web IDL, but
+  not for broken links or references to discontinued specs (see "cc" flag in
+  the definitions of anomalies in src/lib/study.js).
+
+  The option is ignored if the --issues option is not set.
+
 -f, --format <format>
   Tell Strudy to return a report in the specified format. Format may be one of
   "markdown" (default when option is not set) or "json".
@@ -301,7 +318,8 @@ Format must be one of "json" or "markdown".`)
         'json' :
         (options.issues ? 'issue' : 'full'),
       trResults: trReport?.results ?? [],
-      specs: options.spec
+      specs: options.spec,
+      cc: (options.issues ? options.cc : null)
     });
 
     // Output the structured anomaly report

test/study.js

@@ -209,4 +209,38 @@ See [Dealing with the event loop](https://html.spec.whatwg.org/multipage/webappa
   * [ ] Boo`
     });
   });
+
+  it('adds Cc message when requested and when anomaly warrants it', async function() {
+    const crawlResult = [
+      populateSpec(specUrl, {
+        algorithms: [{
+          html: 'The <code class="idl"><a data-link-type="idl" href="https://w3c.github.io/media-capabilities/#dom-mediacapabilities-encodinginfo" id="ref-for-dom-mediacapabilities-encodinginfo">encodingInfo()</a></code> method MUST run the following steps:',
+          rationale: 'if',
+          steps: [
+            { html: 'Let <var>p</var> be a new promise.' },
+            { html: '<a data-link-type="dfn" href="https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel" id="ref-for-in-parallel①">In parallel</a>, run the <a data-link-type="dfn" href="https://w3c.github.io/media-capabilities/#create-a-mediacapabilitiesencodinginfo" id="ref-for-create-a-mediacapabilitiesencodinginfo">Create a MediaCapabilitiesEncodingInfo</a> algorithm with <var>configuration</var> and resolve <var>p</var> with its result.' },
+            { html: 'Return <var>p</var>.' }
+          ]
+        }]
+      }),
+      populateSpec(specUrl2, { error: 'Borked' })
+    ];
+    const report = await study(crawlResult, { cc: ['tidoust'], htmlFragments: {} });
+    assertNbAnomalies(report.results, 2);
+    assertAnomaly(report.results, 0, {
+      title: 'Crawl error in Hello universe API',
+      content:
+`While crawling [Hello universe API](${specUrl2}), the following crawl errors occurred:
+* [ ] Borked`
+    });
+    assertAnomaly(report.results, 1, {
+      title: 'Missing tasks in parallel steps in Hello world API',
+      content: `While crawling [Hello world API](https://w3c.github.io/world/), the following algorithms fire an event, or resolve or reject a Promise, within a step that runs [in parallel](https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel) without first queuing a task:
+* [ ] The algorithm that starts with \"The encodingInfo() method MUST run the following steps:\" resolves/rejects a promise directly in a step that runs in parallel
+
+See [Dealing with the event loop](https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-for-spec-authors) in the HTML specification for guidance on how to deal with algorithm sections that run *in parallel*.
+
+<sub>Cc @tidoust</sub>`
+    });
+  });
 });