From ceb9011d16f2f2f451b5ea1aee23e70992e2d838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 24 Nov 2022 15:30:48 +0100 Subject: [PATCH] Add new PR title check for GitHub issues and GitHub issue validation --- .github/workflows/dev_pr.yml | 4 +- .github/workflows/dev_pr/helpers.js | 55 ++++++++++++++++--- .../dev_pr/{jira_check.js => issue_check.js} | 37 ++++++++++++- .github/workflows/dev_pr/link.js | 25 ++++++++- .github/workflows/dev_pr/title_check.js | 6 +- .github/workflows/dev_pr/title_check.md | 12 ++-- 6 files changed, 116 insertions(+), 23 deletions(-) rename .github/workflows/dev_pr/{jira_check.js => issue_check.js} (68%) diff --git a/.github/workflows/dev_pr.yml b/.github/workflows/dev_pr.yml index f3c59ad54f8f1..cdafa085cab9b 100644 --- a/.github/workflows/dev_pr.yml +++ b/.github/workflows/dev_pr.yml @@ -66,7 +66,7 @@ jobs: const script = require(`${process.env.GITHUB_WORKSPACE}/.github/workflows/dev_pr/title_check.js`); script({github, context}); - - name: Check Jira Issue + - name: Check Issue if: | (github.event.action == 'opened' || github.event.action == 'edited') @@ -75,7 +75,7 @@ jobs: debug: true github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const script = require(`${process.env.GITHUB_WORKSPACE}/.github/workflows/dev_pr/jira_check.js`); + const script = require(`${process.env.GITHUB_WORKSPACE}/.github/workflows/dev_pr/issue_check.js`); script({github, context}); - name: Assign GitHub labels diff --git a/.github/workflows/dev_pr/helpers.js b/.github/workflows/dev_pr/helpers.js index d5f275d27f19a..39d164f319230 100644 --- a/.github/workflows/dev_pr/helpers.js +++ b/.github/workflows/dev_pr/helpers.js @@ -18,19 +18,22 @@ 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 ID of the JIRA or GitHub issue * @param {String} title - * @returns {String} the ID of the associated JIRA issue + * @returns {String} the ID of the associated JIRA or GitHub issue */ -function detectJIRAID(title) { +function detectIssueID(title) { if (!title) { return null; } const matched = /^(WIP:?\s*)?((ARROW|PARQUET)-\d+)/.exec(title); - if (!matched) { - return null; + const matched_gh = /^(WIP:?\s*)?(GH-)(\d+)/.exec(title); + if (matched) { + return matched[2]; + } else if (matched_gh) { + return matched_gh[3] } - return matched[2]; + return null; } /** @@ -69,8 +72,44 @@ async function getJiraInfo(jiraID) { }); } +/** + * Retrieves information about a GitHub issue. + * @param {String} issueID + * @returns {Object} the information about a GitHub issue. + */ + async function getGitHubInfo(github, context, issueID, pullRequestNumber) { + try { + const response = await github.issues.get({ + issue_number: issueID, + owner: context.repo.owner, + repo: context.repo.repo, + }) + return response.data + } catch (error) { + console.log(`${error.name}: ${error.code}`); + return false + } +} + +/** + * 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 = { - detectJIRAID, + detectIssueID, haveJIRAID, - getJiraInfo + getJiraInfo, + haveGitHubIssueID, + getGitHubInfo }; \ No newline at end of file diff --git a/.github/workflows/dev_pr/jira_check.js b/.github/workflows/dev_pr/issue_check.js similarity index 68% rename from .github/workflows/dev_pr/jira_check.js rename to .github/workflows/dev_pr/issue_check.js index 3c294f8c7a0fa..0bdd6428d1a81 100644 --- a/.github/workflows/dev_pr/jira_check.js +++ b/.github/workflows/dev_pr/issue_check.js @@ -78,11 +78,42 @@ async function commentNotStartedTicket(github, context, pullRequestNumber) { } } +async function verifyGitHubIssue(github, context, pullRequestNumber, issueID) { + const issueInfo = await helpers.getGitHubInfo(github, context, issueID, pullRequestNumber); + if (issueInfo) { + if (!issueInfo.assignees.length) { + 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." + }) + } + 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({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pullRequestNumber, + body: ":warning: 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 jiraID = helpers.detectJIRAID(title); - if (jiraID) { - await verifyJIRAIssue(github, context, pullRequestNumber, jiraID); + 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); } }; diff --git a/.github/workflows/dev_pr/link.js b/.github/workflows/dev_pr/link.js index 404ff46436f57..520eac0ee4df1 100644 --- a/.github/workflows/dev_pr/link.js +++ b/.github/workflows/dev_pr/link.js @@ -51,11 +51,30 @@ async function commentJIRAURL(github, context, pullRequestNumber, jiraID) { }); } +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); + // TODO: Check if comment is already there + //if (await haveComment(github, context, pullRequestNumber, jiraURL)) { + // return; + //} + if (issueInfo){ + await github.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pullRequestNumber, + body: "* Github Issue: #" + issueInfo.number + }); + } +} + module.exports = async ({github, context}) => { const pullRequestNumber = context.payload.number; const title = context.payload.pull_request.title; - const jiraID = helpers.detectJIRAID(title); - if (jiraID) { - await commentJIRAURL(github, context, pullRequestNumber, jiraID); + 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); } }; diff --git a/.github/workflows/dev_pr/title_check.js b/.github/workflows/dev_pr/title_check.js index 392108269d8c6..1261e9906fc15 100644 --- a/.github/workflows/dev_pr/title_check.js +++ b/.github/workflows/dev_pr/title_check.js @@ -18,7 +18,7 @@ const fs = require("fs"); const helpers = require("./helpers.js"); -async function commentOpenJIRAIssue(github, context, pullRequestNumber) { +async function commentOpenGitHubIssue(github, context, pullRequestNumber) { const {data: comments} = await github.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, @@ -41,7 +41,7 @@ async function commentOpenJIRAIssue(github, context, pullRequestNumber) { module.exports = async ({github, context}) => { const pullRequestNumber = context.payload.number; const title = context.payload.pull_request.title; - if (!helpers.haveJIRAID(title)) { - await commentOpenJIRAIssue(github, context, pullRequestNumber); + if (!helpers.haveJIRAID(title) || !helpers.haveGitHubIssueID(title)) { + await commentOpenGitHubIssue(github, context, pullRequestNumber); } }; diff --git a/.github/workflows/dev_pr/title_check.md b/.github/workflows/dev_pr/title_check.md index 1db9fcf637bb0..05501eb1300b6 100644 --- a/.github/workflows/dev_pr/title_check.md +++ b/.github/workflows/dev_pr/title_check.md @@ -19,18 +19,22 @@ Thanks for opening a pull request! -If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW +If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose -Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project. +Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project. -Then could you also rename pull request title in the following format? +Then could you also rename the pull request title in the following format? - ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY} + GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY} or MINOR: [${COMPONENT}] ${SUMMARY} +In the case of old issues on JIRA the title also supports: + + ARROW-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY} + See also: * [Other pull requests](https://github.com/apache/arrow/pulls/)