Skip to content

Commit 00f2f8d

Browse files
authored
Only require two approvals for changes to doc/, library/ or verifast-proofs/ (model-checking#351)
We are making a lot of changes to our `scripts/` and `.github/workflows/` folders to improve automation. These changes are low-risk; we have no plans to upstream them and they're not part of the public-facing challenge effort. Update our workflow to require two committee approvals iff `doc/`, `library/` or `verifast-proofs/` are modified, and otherwise just require one. See carolynzech#35 and carolynzech#36 for test runs on my fork. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
1 parent 0700c36 commit 00f2f8d

File tree

1 file changed

+23
-4
lines changed

1 file changed

+23
-4
lines changed

.github/workflows/pr_approval.yml

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,21 @@ jobs:
7373
return;
7474
}
7575
76+
// Get the list of changed files
77+
const { data: changedFiles } = await github.rest.pulls.listFiles({
78+
owner,
79+
repo,
80+
pull_number,
81+
});
82+
83+
// Check if any files in doc/, library/ or verifast-proofs/ are modified
84+
const affectsDocs = changedFiles.some(file => file.filename.startsWith('doc/'));
85+
const affectsLibrary = changedFiles.some(file => file.filename.startsWith('library/'));
86+
const affectsVerifast = changedFiles.some(file => file.filename.startsWith('verifast-proofs/'));
87+
// Require two approvals iff one of the above folders are modified; otherwise, one is sufficient.
88+
const requiresTwoApprovals = affectsDocs || affectsLibrary || affectsVerifast;
89+
const requiredApprovals = requiresTwoApprovals ? 2 : 1;
90+
7691
// Get all reviews with pagination
7792
async function getAllReviews() {
7893
let allReviews = [];
@@ -113,23 +128,27 @@ jobs:
113128
.map(review => review.user.login)
114129
);
115130
116-
const requiredApprovals = 2;
117131
const committeeApprovers = Array.from(approvers)
118132
.filter(approver => requiredApprovers.includes(approver));
119133
const currentCountfromCommittee = committeeApprovers.length;
120134
121-
// Core logic that checks if the approvers are in the committee
122-
const conclusion = (currentCountfromCommittee >= 2) ? 'success' : 'failure';
135+
// Check if we have enough approvals
136+
const conclusion = (currentCountfromCommittee >= requiredApprovals) ? 'success' : 'failure';
123137
124138
console.log('PR Approval Status');
139+
console.log('Modified folders:');
140+
console.log(`- doc/: ${affectsDocs ? 'yes' : 'no'}`);
141+
console.log(`- library/: ${affectsLibrary ? 'yes' : 'no'}`);
142+
console.log(`- verifast-proofs/: ${affectsVerifast ? 'yes' : 'no'}`);
143+
console.log(`Required approvals from committee: ${requiredApprovals}`);
125144
console.log(`PR has ${approvers.size} total approvals and ${currentCountfromCommittee} required approvals from the committee.`);
126145
127146
console.log(`Committee Members: [${requiredApprovers.join(', ')}]`);
128147
console.log(`Committee Approvers: ${committeeApprovers.length === 0 ? 'NONE' : `[${committeeApprovers.join(', ')}]`}`);
129148
console.log(`All Approvers: ${approvers.size === 0 ? 'NONE' : `[${Array.from(approvers).join(', ')}]`}`);
130149
131150
if (conclusion === 'failure') {
132-
core.setFailed(`PR needs 2 approvals from committee members, but it has ${currentCountfromCommittee}`);
151+
core.setFailed(`PR needs ${requiredApprovals} approval${requiredApprovals > 1 ? 's' : ''} from committee members, but it has ${currentCountfromCommittee}`);
133152
} else {
134153
core.info('PR approval check passed successfully.');
135154
}

0 commit comments

Comments
 (0)