From 0568983377a435419d24dd2f99a21dc0c10d913d Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 14:58:41 +0000 Subject: [PATCH 01/15] modify pr workflow --- .github/workflows/pr_approval.yml | 32 +++++++++---------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index fbac5035e2d3a..5bd82ae3e4e16 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -132,29 +132,15 @@ jobs: check_name: checkName }); - // Reuse the same workflow everytime there's a new review submitted - // instead of creating new workflows. Better efficiency and readability - // as the number of workflows is kept to a minimal number - if (checkRuns.data.total_count > 0) { - await github.rest.checks.update({ - owner, - repo, - check_run_id: checkRuns.data.check_runs[0].id, - status: 'completed', - conclusion, - output - }); - } else { - await github.rest.checks.create({ - owner, - repo, - name: checkName, - head_sha: pr.data.head.sha, - status: 'completed', - conclusion, - output - }); - } + await github.rest.checks.create({ + owner, + repo, + name: checkName, + head_sha: pr.data.head.sha, + status: 'completed', + conclusion, + output + }); if (conclusion === 'failure') { core.setFailed(`PR needs at least ${requiredApprovals} total approvals and 2 required approvals. Current approvals: ${approvers.size}, Required approvals: ${requiredApprovals}`); From 2c323e1d01dc487789e464c4401d33fa540ffb7e Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 14:59:43 +0000 Subject: [PATCH 02/15] add dummy account to toml for testing --- .github/pull_requests.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/pull_requests.toml b/.github/pull_requests.toml index 12113728fc722..f7c83f9647284 100644 --- a/.github/pull_requests.toml +++ b/.github/pull_requests.toml @@ -9,5 +9,6 @@ members = [ "remi-delmas-3000", "qinheping", "tautschnig", - "jaisnan" + "jaisnan", + "Jaisu-1" ] From a25a44915f27260c62f35863f226b0d54233d372 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:06:34 +0000 Subject: [PATCH 03/15] add workflow --- .github/workflows/pr_approval.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 5bd82ae3e4e16..c10205d80babc 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -58,7 +58,7 @@ jobs: pull_number }); - const relevantPaths = ['library/', 'doc/src/challenges/']; + const relevantPaths = ['library/', 'doc/src/challenges/', '.github/workflows']; const isRelevantPR = files.data.some(file => relevantPaths.some(path => file.filename.startsWith(path)) ); From d31df34794295e521868bc27bd220aa2221800c0 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:12:47 +0000 Subject: [PATCH 04/15] remove extra console log --- .github/workflows/pr_approval.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index c10205d80babc..6bad8de9dedc5 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -73,7 +73,6 @@ jobs: const tomlContent = fs.readFileSync('.github/pull_requests.toml', 'utf8'); console.log('TOML content:', tomlContent); const tomlData = toml.parse(tomlContent); - console.log('Parsed TOML data:', JSON.stringify(tomlData, null, 2)); if (!tomlData.committee || !Array.isArray(tomlData.committee.members)) { throw new Error('committee.members is not an array in the TOML file'); @@ -124,7 +123,7 @@ jobs: pull_number }); - // Create or update check run + // Update check run const checkRuns = await github.rest.checks.listForRef({ owner, repo, From aae2423e5610bc9ee7bd85a962dec95a7bda16c4 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:13:31 +0000 Subject: [PATCH 05/15] check with one --- .github/workflows/pr_approval.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 6bad8de9dedc5..329cf5b43bae2 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -98,7 +98,7 @@ jobs: .map(review => review.user.login) ); - const requiredApprovals = 2; + const requiredApprovals = 1; const currentCountfromCommittee = Array.from(approvers) .filter(approver => requiredApprovers.includes(approver)) .length; From ceb21e7e52c390370bfa863daa53e917b293d493 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:17:14 +0000 Subject: [PATCH 06/15] remove duplicate logic --- .github/workflows/pr_approval.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 329cf5b43bae2..87469c161fb74 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -109,7 +109,7 @@ jobs: // Core logic that checks if the approvers are in the committee const checkName = 'PR Approval Status'; - const conclusion = (approvers.size >= requiredApprovals && currentCountfromCommittee >= 2) ? 'success' : 'failure'; + const conclusion = (currentCountfromCommittee >= requiredApprovals) ? 'success' : 'failure'; const output = { title: checkName, summary: `PR has ${approvers.size} total approvals and ${requiredApprovals} required approvals.`, From 3f530a4d512cc8bac2da5ebcdeaf8168a2390a15 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:37:25 +0000 Subject: [PATCH 07/15] add @carolynzech to approvers list for testing --- .github/pull_requests.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/pull_requests.toml b/.github/pull_requests.toml index f7c83f9647284..e0b80e637d0d5 100644 --- a/.github/pull_requests.toml +++ b/.github/pull_requests.toml @@ -10,5 +10,6 @@ members = [ "qinheping", "tautschnig", "jaisnan", - "Jaisu-1" + "Jaisu-1", + "carolynzech" ] From 0fd93bf1d79fb3d7844cc325b38a788a57819bb2 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:45:12 +0000 Subject: [PATCH 08/15] Revert requirement back to 1 --- .github/pull_requests.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/pull_requests.toml b/.github/pull_requests.toml index e0b80e637d0d5..387da5d9f8f74 100644 --- a/.github/pull_requests.toml +++ b/.github/pull_requests.toml @@ -10,6 +10,5 @@ members = [ "qinheping", "tautschnig", "jaisnan", - "Jaisu-1", "carolynzech" ] From 233661de1957c70db36711fca5324177279cec96 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:47:05 +0000 Subject: [PATCH 09/15] Change approvers to 2 --- .github/workflows/pr_approval.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 87469c161fb74..11aed86154cb8 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -98,7 +98,7 @@ jobs: .map(review => review.user.login) ); - const requiredApprovals = 1; + const requiredApprovals = 2; const currentCountfromCommittee = Array.from(approvers) .filter(approver => requiredApprovers.includes(approver)) .length; From 22e770b46c5a8db5e03408982e9e66e5c0e796b9 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 15:48:16 +0000 Subject: [PATCH 10/15] resolve conflict --- .github/pull_requests.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/pull_requests.toml b/.github/pull_requests.toml index 387da5d9f8f74..7faf67ed5db0b 100644 --- a/.github/pull_requests.toml +++ b/.github/pull_requests.toml @@ -10,5 +10,6 @@ members = [ "qinheping", "tautschnig", "jaisnan", + "patricklam", "carolynzech" ] From 5ac231eef79810a2f34bb2612b43d2b336eebdce Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 16:48:36 +0000 Subject: [PATCH 11/15] remove github createa --- .github/workflows/pr_approval.yml | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 11aed86154cb8..9aa74464b0e25 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -116,31 +116,9 @@ jobs: text: `Approvers: ${Array.from(approvers).join(', ')}\nRequired Approvers: ${requiredApprovers.join(', ')}` }; - // Get PR details - const pr = await github.rest.pulls.get({ - owner, - repo, - pull_number - }); - - // Update check run - const checkRuns = await github.rest.checks.listForRef({ - owner, - repo, - ref: pr.data.head.sha, - check_name: checkName - }); - - await github.rest.checks.create({ - owner, - repo, - name: checkName, - head_sha: pr.data.head.sha, - status: 'completed', - conclusion, - output - }); - + // Simplify to not use github create at all if (conclusion === 'failure') { core.setFailed(`PR needs at least ${requiredApprovals} total approvals and 2 required approvals. Current approvals: ${approvers.size}, Required approvals: ${requiredApprovals}`); + } else { + console.log(`PR has sufficient approvals (${approvers.size}/${requiredApprovals})`); } From 6cac23c0bba4c99a94d2cc0afc27e7ca966f3717 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 16:54:41 +0000 Subject: [PATCH 12/15] update trigger condition --- .github/workflows/pr_approval.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 9aa74464b0e25..9b79d6055f0d7 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -5,18 +5,12 @@ name: Check PR Approvals # To protect against that scenario, we can turn on number of approvals required to 2 in the github settings # of the repository on: - pull_request_review: - types: [submitted] workflow_dispatch: - -# Without these permissions, we get a 403 error from github -# for trying to modify the pull request for newer project. -# Source: https://stackoverflow.com/a/76994510 -permissions: write-all + pull_request: + branches: [ main ] jobs: check-approvals: - if: github.event.review.state == 'APPROVED' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest steps: - name: Checkout repository From b079d37c011ffbdef8bc6ffa1458ee890605c189 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 17:05:12 +0000 Subject: [PATCH 13/15] Re-run on pull and approval as well --- .github/workflows/pr_approval.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 9b79d6055f0d7..92d1d6a74de4c 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -8,6 +8,8 @@ on: workflow_dispatch: pull_request: branches: [ main ] + pull_request_review: + types: [submitted] jobs: check-approvals: From efc68d02fabb80362259640a7456bc043a7cd329 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 17:58:35 +0000 Subject: [PATCH 14/15] try with edited pull_request trigger --- .github/workflows/pr_approval.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 92d1d6a74de4c..bd4c6525ca3da 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -7,9 +7,10 @@ name: Check PR Approvals on: workflow_dispatch: pull_request: + types: + - opened + - edited branches: [ main ] - pull_request_review: - types: [submitted] jobs: check-approvals: From 9557879f9b06d537327c299eb7c57464a6807731 Mon Sep 17 00:00:00 2001 From: Jaisurya Nanduri Date: Thu, 22 Aug 2024 18:25:07 +0000 Subject: [PATCH 15/15] add push paths as well --- .github/workflows/pr_approval.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index bd4c6525ca3da..1ded62522cf5e 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -7,10 +7,14 @@ name: Check PR Approvals on: workflow_dispatch: pull_request: - types: - - opened - - edited branches: [ main ] + pull_request_review: + types: [submitted] + push: + paths: + - 'library/**' + - 'doc/src/challenges/' + - '.github/workflows' jobs: check-approvals: