Review comments 1

raulcd · Nov 25, 2022 · bc4a07a · bc4a07a
1 parent 99c2cc2
commit bc4a07a
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 57 deletions.
diff --git a/.github/workflows/dev_pr/helpers.js b/.github/workflows/dev_pr/helpers.js
@@ -18,37 +18,33 @@
 const https = require('https');
 
 /**
- * Given the title of a PullRequest return the ID of the JIRA or GitHub issue
+ * Given the title of a PullRequest return the Issue
+ *
  * @param {String} title 
- * @returns {String} the ID of the associated JIRA or GitHub issue
+ * @returns {Issue} or null if no issue detected.
+ *
+ * @typedef {Object} Issue
+ * @property {string} kind - The kind of issue: minor, jira or github
+ * @property {string} id   - The id of the issue:
+ *                            ARROW-XXXX, PARQUET-XXXX for jira
+ *                            The numeric issue id for github
  */
-function detectIssueID(title) {
+function detectIssue(title) {
     if (!title) {
         return null;
     }
-    const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
-    const matched_gh = /^(WIP:?\s*)?(GH-)(\d+)/.exec(title);
-    if (matched) {
-        return matched[2];
-    } else if (matched_gh) {
-        return matched_gh[3]
+    if (title.startsWith("MINOR: ")) {
+        return {"kind": "minor"};
     }
-    return null;
-}
-
-/**
- * Given the title of a PullRequest checks if it contains a JIRA issue ID
- * @param {String} title 
- * @returns {Boolean} true if it starts with a JIRA ID or MINOR:
- */
-function haveJIRAID(title) {
-    if (!title) {
-      return false;
+    const matched_jira = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
+    if (matched_jira) {
+        return {"kind": "jira", "id": matched_jira[2]};
     }
-    if (title.startsWith("MINOR: ")) {
-      return true;
+    const matched_gh = /^(WIP:?\s*)?(GH-)(\d+)/.exec(title);
+    if (matched_gh) {
+        return {"kind": "github", "id": matched_gh[3]};
     }
-    return /^(WIP:?\s*)?(ARROW|PARQUET)-\d+/.test(title);
+    return null;
 }
 
 /**
@@ -91,25 +87,8 @@ async function getJiraInfo(jiraID) {
     }
 }
 
-/**
- * Given the title of a PullRequest checks if it contains a GitHub issue ID
- * @param {String} title
- * @returns {Boolean} true if title starts with a GitHub ID or MINOR:
- */
- function haveGitHubIssueID(title) {
-    if (!title) {
-      return false;
-    }
-    if (title.startsWith("MINOR: ")) {
-      return true;
-    }
-    return /^(WIP:?\s*)?(GH)-\d+/.test(title);
-}
-
 module.exports = {
-    detectIssueID,
-    haveJIRAID,
+    detectIssue,
     getJiraInfo,
-    haveGitHubIssueID,
     getGitHubInfo
 };
diff --git a/.github/workflows/dev_pr/issue_check.js b/.github/workflows/dev_pr/issue_check.js
@@ -82,11 +82,12 @@ async function verifyGitHubIssue(github, context, pullRequestNumber, issueID) {
     const issueInfo = await helpers.getGitHubInfo(github, context, issueID, pullRequestNumber);
     if (issueInfo) {
         if (!issueInfo.assignees.length) {
+            //await assignGitHubIssue(github, context, pullRequestNumber, issueID);
             await github.issues.createComment({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 issue_number: pullRequestNumber,
-                body: ":warning: GitHub issue #" + issueID + " **has not been assigned in GitHub**, please assign the ticket."
+                body: ":warning: GitHub issue #" + issueID + " **has been automatically assigned in GitHub** to PR creator."
             })
         }
         if(!issueInfo.labels.length) {
@@ -102,18 +103,18 @@ async function verifyGitHubIssue(github, context, pullRequestNumber, issueID) {
             owner: context.repo.owner,
             repo: context.repo.repo,
             issue_number: pullRequestNumber,
-            body: ":warning: GitHub issue #" + issueID + " could not be retrieved."
+            body: ":x: GitHub issue #" + issueID + " could not be retrieved."
         })
     }
 }
 
 module.exports = async ({github, context}) => {
     const pullRequestNumber = context.payload.number;
     const title = context.payload.pull_request.title;
-    const issueID = helpers.detectIssueID(title)
-    if (helpers.haveJIRAID(title)) {
-          await verifyJIRAIssue(github, context, pullRequestNumber, issueID);
-    } else if(helpers.haveGitHubIssueID(title)) {
-        await verifyGitHubIssue(github, context, pullRequestNumber, issueID);
+    const issue = helpers.detectIssue(title)
+    if (issue.kind == "jira") {
+          await verifyJIRAIssue(github, context, pullRequestNumber, issue.id);
+    } else if(issue.kind == "github") {
+        await verifyGitHubIssue(github, context, pullRequestNumber, issue.id);
     }
 };
diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js
@@ -18,7 +18,16 @@
 const helpers = require("./helpers.js");
 
 
-async function haveComment(github, context, pullRequestNumber, body) {
+/**
+ * Checks whether message is present on Pull Request list of comments.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {String} message
+ * @returns {Boolean} true if message was found.
+ */
+async function haveComment(github, context, pullRequestNumber, message) {
   const options = {
     owner: context.repo.owner,
     repo: context.repo.repo,
@@ -27,7 +36,7 @@ async function haveComment(github, context, pullRequestNumber, body) {
   };
   while (true) {
     const response = await github.issues.listComments(options);
-    if (response.data.some(comment => comment.body === body)) {
+    if (response.data.some(comment => comment.body === message)) {
       return true;
     }
     if (!/;\s*rel="next"/.test(response.headers.link || "")) {
@@ -38,6 +47,14 @@ async function haveComment(github, context, pullRequestNumber, body) {
   return false;
 }
 
+/**
+ * Adds a comment on the Pull Request linking the JIRA issue.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {String} jiraID
+ */
 async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
   const jiraURL = `https://issues.apache.org/jira/browse/${jiraID}`;
   if (await haveComment(github, context, pullRequestNumber, jiraURL)) {
@@ -51,10 +68,18 @@ async function commentJIRAURL(github, context, pullRequestNumber, jiraID) {
   });
 }
 
+/**
+ * Adds a comment on the Pull Request linking the GitHub issue.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber - String containing numeric id of PR
+ * @param {String} issueID - String containing numeric id of the github issue
+ */
 async function commentGitHubURL(github, context, pullRequestNumber, issueID) {
   // Make the call to ensure issue exists before adding comment
   const issueInfo = await helpers.getGitHubInfo(github, context, issueID, pullRequestNumber);
-  const message = "* Github Issue: #" + issueInfo.number
+  const message = "Closes #" + issueInfo.number
   if (await haveComment(github, context, pullRequestNumber, message)) {
     return;
   }
@@ -70,11 +95,15 @@ async function commentGitHubURL(github, context, pullRequestNumber, issueID) {
 
 module.exports = async ({github, context}) => {
   const pullRequestNumber = context.payload.number;
+  console.log("Payload")
+  console.log(context.payload)
+  console.log("Pull Request")
+  console.log(context.payload.pull_request)
   const title = context.payload.pull_request.title;
-  const issueID = helpers.detectIssueID(title);
-  if (helpers.haveJIRAID(title)) {
-    await commentJIRAURL(github, context, pullRequestNumber, issueID);
-  } else if (helpers.haveGitHubIssueID(title)) {
-    await commentGitHubURL(github, context, pullRequestNumber, issueID);
+  const issue = helpers.detectIssue(title);
+  if (issue.kind == "jira") {
+    await commentJIRAURL(github, context, pullRequestNumber, issue.id);
+  } else if (issue.kind == "github") {
+    await commentGitHubURL(github, context, pullRequestNumber, issue.id);
   }
 };
diff --git a/.github/workflows/dev_pr/title_check.js b/.github/workflows/dev_pr/title_check.js
@@ -41,7 +41,8 @@ async function commentOpenGitHubIssue(github, context, pullRequestNumber) {
 module.exports = async ({github, context}) => {
   const pullRequestNumber = context.payload.number;
   const title = context.payload.pull_request.title;
-  if (!helpers.haveJIRAID(title) || !helpers.haveGitHubIssueID(title)) {
+  const issue = helpers.detectIssue(title)
+  if (!issue) {
     await commentOpenGitHubIssue(github, context, pullRequestNumber);
   }
 };
diff --git a/.github/workflows/dev_pr/title_check.md b/.github/workflows/dev_pr/title_check.md
@@ -34,6 +34,7 @@ or
 In the case of old issues on JIRA the title also supports:
 
     ARROW-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
+    PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
 
 See also: