From 5f4eafc0f68fbff45fc582abb85de06b9762580b Mon Sep 17 00:00:00 2001 From: Carolyn Zech Date: Wed, 7 May 2025 11:43:25 -0400 Subject: [PATCH 1/3] Require 2 approvals iff library/ files are modified --- .github/workflows/pr_approval.yml | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index 70089fed5ec3f..4d902c83bd08e 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -73,6 +73,17 @@ jobs: return; } + // Get the list of changed files + const { data: changedFiles } = await github.rest.pulls.listFiles({ + owner, + repo, + pull_number, + }); + + // Check if any files in library/ are modified + const affectsLibrary = changedFiles.some(file => file.filename.startsWith('library/')); + const requiredApprovals = affectsLibrary ? 2 : 1; + // Get all reviews with pagination async function getAllReviews() { let allReviews = []; @@ -113,15 +124,16 @@ jobs: .map(review => review.user.login) ); - const requiredApprovals = 2; const committeeApprovers = Array.from(approvers) .filter(approver => requiredApprovers.includes(approver)); const currentCountfromCommittee = committeeApprovers.length; - // Core logic that checks if the approvers are in the committee - const conclusion = (currentCountfromCommittee >= 2) ? 'success' : 'failure'; + // Check if we have enough approvals + const conclusion = (currentCountfromCommittee >= requiredApprovals) ? 'success' : 'failure'; console.log('PR Approval Status'); + console.log(`PR ${affectsLibrary ? 'affects' : 'does not affect'} library/ files`); + console.log(`Required approvals from committee: ${requiredApprovals}`); console.log(`PR has ${approvers.size} total approvals and ${currentCountfromCommittee} required approvals from the committee.`); console.log(`Committee Members: [${requiredApprovers.join(', ')}]`); @@ -129,7 +141,7 @@ jobs: console.log(`All Approvers: ${approvers.size === 0 ? 'NONE' : `[${Array.from(approvers).join(', ')}]`}`); if (conclusion === 'failure') { - core.setFailed(`PR needs 2 approvals from committee members, but it has ${currentCountfromCommittee}`); + core.setFailed(`PR needs ${requiredApprovals} approval${requiredApprovals > 1 ? 's' : ''} from committee members, but it has ${currentCountfromCommittee}`); } else { core.info('PR approval check passed successfully.'); } From 8287404cb7227f50587d5a69dec48ffa02ce92e4 Mon Sep 17 00:00:00 2001 From: Carolyn Zech Date: Wed, 7 May 2025 12:00:35 -0400 Subject: [PATCH 2/3] also require two approvals for verifast changes --- .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 4d902c83bd08e..adf5f9cbfda02 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -80,9 +80,11 @@ jobs: pull_number, }); - // Check if any files in library/ are modified + // Check if any files in library/ or verifast-proofs/ are modified const affectsLibrary = changedFiles.some(file => file.filename.startsWith('library/')); - const requiredApprovals = affectsLibrary ? 2 : 1; + const affectsVerifast = changedFiles.some(file => file.filename.startsWith('verifast-proofs/')); + const requiresTwoApprovals = affectsLibrary || affectsVerifast; + const requiredApprovals = requiresTwoApprovals ? 2 : 1; // Get all reviews with pagination async function getAllReviews() { @@ -132,7 +134,9 @@ jobs: const conclusion = (currentCountfromCommittee >= requiredApprovals) ? 'success' : 'failure'; console.log('PR Approval Status'); - console.log(`PR ${affectsLibrary ? 'affects' : 'does not affect'} library/ files`); + console.log('Modified folders:'); + console.log(`- library/: ${affectsLibrary ? 'yes' : 'no'}`); + console.log(`- verifast-proofs/: ${affectsVerifast ? 'yes' : 'no'}`); console.log(`Required approvals from committee: ${requiredApprovals}`); console.log(`PR has ${approvers.size} total approvals and ${currentCountfromCommittee} required approvals from the committee.`); From e61675813022aff2fa979c8242ff9c3a8db62c08 Mon Sep 17 00:00:00 2001 From: Carolyn Zech Date: Thu, 8 May 2025 09:57:42 -0400 Subject: [PATCH 3/3] require two approvals for documentation changes --- .github/workflows/pr_approval.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr_approval.yml b/.github/workflows/pr_approval.yml index adf5f9cbfda02..a244de66c6097 100644 --- a/.github/workflows/pr_approval.yml +++ b/.github/workflows/pr_approval.yml @@ -80,10 +80,12 @@ jobs: pull_number, }); - // Check if any files in library/ or verifast-proofs/ are modified + // Check if any files in doc/, library/ or verifast-proofs/ are modified + const affectsDocs = changedFiles.some(file => file.filename.startsWith('doc/')); const affectsLibrary = changedFiles.some(file => file.filename.startsWith('library/')); const affectsVerifast = changedFiles.some(file => file.filename.startsWith('verifast-proofs/')); - const requiresTwoApprovals = affectsLibrary || affectsVerifast; + // Require two approvals iff one of the above folders are modified; otherwise, one is sufficient. + const requiresTwoApprovals = affectsDocs || affectsLibrary || affectsVerifast; const requiredApprovals = requiresTwoApprovals ? 2 : 1; // Get all reviews with pagination @@ -135,6 +137,7 @@ jobs: console.log('PR Approval Status'); console.log('Modified folders:'); + console.log(`- doc/: ${affectsDocs ? 'yes' : 'no'}`); console.log(`- library/: ${affectsLibrary ? 'yes' : 'no'}`); console.log(`- verifast-proofs/: ${affectsVerifast ? 'yes' : 'no'}`); console.log(`Required approvals from committee: ${requiredApprovals}`);