amol- commented on code in PR #14731:
URL: https://github.com/apache/arrow/pull/14731#discussion_r1033645987
##########
.github/workflows/dev_pr/helpers.js:
##########
@@ -18,34 +18,33 @@
const https = require('https');
/**
- * Given the title of a PullRequest return the ID of the JIRA issue
+ * Given the title of a PullRequest return the Issue
+ *
* @param {String} title
- * @returns {String} the ID of the associated JIRA 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 detectJIRAID(title) {
+function detectIssue(title) {
if (!title) {
return null;
}
- const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title);
- if (!matched) {
- return null;
+ if (title.startsWith("MINOR: ")) {
+ return {"kind": "minor"};
}
- return matched[2];
-}
-
-/**
- * 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]};
Review Comment:
I think you can remove the brackets around `GH-` I don't see the value of
retrieving them as a match group. (At that point `matched_gh[3]` should become
`matched_gh[2]`)
##########
.github/workflows/dev_pr/issue_check.js:
##########
@@ -78,11 +102,73 @@ async function commentNotStartedTicket(github, context,
pullRequestNumber) {
}
}
+/**
+ * Assigns the Github Issue to the PR creator.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {Object} issueInfo
+ */
+async function assignGitHubIssue(github, context, pullRequestNumber,
issueInfo) {
+ await github.issues.addAssignees({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: issueInfo.number,
+ assignees: context.payload.pull_request.user.login
+ });
+ await github.issues.createComment({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: pullRequestNumber,
+ body: ":warning: GitHub issue #" + issueInfo.number + " **has been
automatically assigned in GitHub** to PR creator."
+ });
+}
+
+/**
+ * Performs checks on the GitHub Issue:
+ * - The issue is assigned to someone. If not assign it gets automatically
+ * assigned to the PR creator.
+ * - The issue contains any label.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {String} issueID
+ */
+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,
issueInfo);
+ }
+ if(!issueInfo.labels.length) {
Review Comment:
This should probably check if there is a `label["name"]` that starts with
`Component:`, I don't think that we want to accept _any_ label.
##########
.github/workflows/dev_pr/issue_check.js:
##########
@@ -78,11 +102,73 @@ async function commentNotStartedTicket(github, context,
pullRequestNumber) {
}
}
+/**
+ * Assigns the Github Issue to the PR creator.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {Object} issueInfo
+ */
+async function assignGitHubIssue(github, context, pullRequestNumber,
issueInfo) {
+ await github.issues.addAssignees({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: issueInfo.number,
+ assignees: context.payload.pull_request.user.login
+ });
+ await github.issues.createComment({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: pullRequestNumber,
+ body: ":warning: GitHub issue #" + issueInfo.number + " **has been
automatically assigned in GitHub** to PR creator."
+ });
+}
+
+/**
+ * Performs checks on the GitHub Issue:
+ * - The issue is assigned to someone. If not assign it gets automatically
+ * assigned to the PR creator.
+ * - The issue contains any label.
+ *
+ * @param {Object} github
+ * @param {Object} context
+ * @param {String} pullRequestNumber
+ * @param {String} issueID
+ */
+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,
issueInfo);
+ }
+ if(!issueInfo.labels.length) {
+ await github.issues.createComment({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: pullRequestNumber,
+ body: ":warning: GitHub issue #" + issueID + " **has no labels
in GitHub**, please add labels for components."
+ })
+ }
+ } else {
+ await github.issues.createComment({
Review Comment:
I suggest we move this as a guard at the begin (`if !issueInfo { ... return:
}`) so that we can reduce the nesting and make code easier to read.
##########
.github/workflows/dev_pr/link.js:
##########
@@ -51,11 +68,40 @@ 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
+ if (await haveComment(github, context, pullRequestNumber, message)) {
+ return;
+ }
+ if (issueInfo){
Review Comment:
I noticed that `commentJIRAURL` does not have the check for `issueInfo`. Is
there a specific reason why we added it here? Should we add it to
`commentJIRAURL` too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]