chore: clean up and comments

src/chromium-review.ts

@@ -5,48 +5,44 @@ export async function handleChromiumReviewUnfurl(url: string): Promise<MessageAt
   const match = /^https:\/\/chromium-review\.googlesource\.com\/c\/([a-z0-9]+)\/([a-z0-9]+)\/\+\/([0-9]+)/g.exec(
     url,
   );
-  if (match) {
-    const repo = `${match[1]}%2F${match[2]}`;
-    const niceRepo = `${match[1]}/${match[2]}`;
-    const cl = parseInt(match[3], 10);
+  if (!match) return null;
 
-    const detailsUrl = `https://chromium-review.googlesource.com/changes/${repo}~${cl}/detail?O=916314`;
-    const detailsResponse = await fetch(detailsUrl);
-    const detailsText = await detailsResponse.text();
-    const details = JSON.parse(detailsText.substr(4));
-    const { project, subject, owner, labels, current_revision, revisions } = details;
-    const commit = revisions[current_revision].commit;
-    const {
-      message,
-      author: { date },
-    } = commit;
-    const messageWithoutSubject = message.startsWith(subject)
-      ? message.substr(subject.length + 1).trim()
-      : message;
+  const repo = `${match[1]}%2F${match[2]}`;
+  const niceRepo = `${match[1]}/${match[2]}`;
+  const cl = parseInt(match[3], 10);
 
-    return {
-      color: '#4D394B',
-      author_name: owner.name,
-      author_icon:
-        owner.avatars && owner.avatars.length
-          ? owner.avatars[owner.avatars.length - 1].url
-          : ':void',
-      author_link: `https://chromium-review.googlesource.com/q/author:${encodeURIComponent(
-        owner.email,
-      )}`,
-      fallback: `[${niceRepo}] #${cl} ${subject}`,
-      title: `#${cl} ${subject}`,
-      title_link: url,
-      footer_icon: 'https://chromium-review.googlesource.com/favicon.ico',
-      text: messageWithoutSubject,
-      footer: `<https://source.chromium.org/chromium/${niceRepo}|${niceRepo}>`,
-      ts: `${new Date(date).getTime()}`,
-      // TODO: Labels? CQ status?
-      // fields: [{
+  const detailsUrl = `https://chromium-review.googlesource.com/changes/${repo}~${cl}/detail?O=916314`;
+  const detailsResponse = await fetch(detailsUrl);
+  const detailsText = await detailsResponse.text();
+  const details = JSON.parse(detailsText.substr(4));
+  const { project, subject, owner, labels, current_revision, revisions } = details;
+  const commit = revisions[current_revision].commit;
+  const {
+    message,
+    author: { date },
+  } = commit;
+  const messageWithoutSubject = message.startsWith(subject)
+    ? message.substr(subject.length + 1).trim()
+    : message;
 
-      // }]
-    };
-  }
+  return {
+    color: '#4D394B',
+    author_name: owner.name,
+    author_icon:
+      owner.avatars && owner.avatars.length ? owner.avatars[owner.avatars.length - 1].url : ':void',
+    author_link: `https://chromium-review.googlesource.com/q/author:${encodeURIComponent(
+      owner.email,
+    )}`,
+    fallback: `[${niceRepo}] #${cl} ${subject}`,
+    title: `#${cl} ${subject}`,
+    title_link: url,
+    footer_icon: 'https://chromium-review.googlesource.com/favicon.ico',
+    text: messageWithoutSubject,
+    footer: `<https://source.chromium.org/chromium/${niceRepo}|${niceRepo}>`,
+    ts: `${new Date(date).getTime()}`,
+    // TODO: Labels? CQ status?
+    // fields: [{
 
-  return null;
+    // }]
+  };
 }

src/crbug.ts

@@ -2,6 +2,7 @@ import { MessageAttachment } from '@slack/bolt';
 import fetch from 'node-fetch';
 
 import { Policy, ConstantBackoff } from 'cockatiel';
+import { notNull } from './utils';
 
 function parseBugIdentifier(url: string) {
   const parsed = new URL(url);
@@ -79,10 +80,6 @@ type MonorailComments = {
   content: string;
 }[];
 
-const notNull = <T>(arr: (T | null)[]) => {
-  return arr.filter(Boolean) as T[];
-};
-
 export async function handleChromiumBugUnfurl(url: string): Promise<MessageAttachment | null> {
   const bugIdentifier = parseBugIdentifier(url);
   if (!bugIdentifier) return null;

src/crsource.ts

@@ -68,6 +68,14 @@ async function getFileContents(
 
   const best = { str: '', n: 0 };
 
+  /**
+   * This is a truly terrible huristic to parse an otherwise constantly
+   * changing data structure
+   *
+   * We assume the "file contents" is the string somewhere in this deeply
+   * nested array that has the most line breaks.  This is statistically
+   * accurate but Feels Bad :tm:
+   */
   function search(arr: DeepArrayOfUnknowns) {
     for (const item of arr) {
       if (typeof item === 'string') {
@@ -88,6 +96,8 @@ async function getFileContents(
 
 const MAX_SLACK_MSG_LENGTH = 7000;
 
+// Slack messages have a max length, Chromium source code does not
+// You see the problem :)
 function maybeTruncate(longContents: string) {
   if (longContents.length <= MAX_SLACK_MSG_LENGTH) return longContents;
   return longContents.slice(0, MAX_SLACK_MSG_LENGTH) + '...';
@@ -103,6 +113,12 @@ function indentLength(line: string): number {
   return i;
 }
 
+/**
+ * This method removes consistent indenting from the front of a subset
+ * of source code.  i.e. if all code is indented by at least 10 spaces
+ * we will remove 10 spaces from the start of every line to ensure that
+ * the code looks reasonable in Slack.
+ */
 function removeOverIndent(contents: string): string {
   const lines = contents.split('\n');
   if (!lines.length) return contents;

src/index.ts

@@ -3,6 +3,7 @@ import { App, MessageAttachment } from '@slack/bolt';
 import { handleChromiumReviewUnfurl } from './chromium-review';
 import { handleChromiumBugUnfurl } from './crbug';
 import { handleChromiumSourceUnfurl } from './crsource';
+import { notNull } from './utils';
 
 const app = new App({
   authorize: async () => ({
@@ -28,7 +29,7 @@ app.event('link_shared', async ({ client, body }) => {
         handleChromiumBugUnfurl(url),
         handleChromiumSourceUnfurl(url),
       ]);
-      const validUnfurls = unfurls.filter((unfurl) => !!unfurl) as MessageAttachment[];
+      const validUnfurls = notNull(unfurls);
       if (validUnfurls.length > 1) {
         console.error('More than one unfurler responded to a given URL', { url });
       } else if (validUnfurls.length === 1) {

src/utils.ts

@@ -0,0 +1,3 @@
+export const notNull = <T>(arr: (T | null)[]) => {
+  return arr.filter(Boolean) as T[];
+};