Skip to content

Modify commitee TOML and workflow #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 19 commits into from
Closed

Modify commitee TOML and workflow #67

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0568983
modify pr workflow
jaisnan Aug 22, 2024
2c323e1
add dummy account to toml for testing
jaisnan Aug 22, 2024
a25a449
add workflow
jaisnan Aug 22, 2024
d31df34
remove extra console log
jaisnan Aug 22, 2024
aae2423
check with one
jaisnan Aug 22, 2024
ceb21e7
remove duplicate logic
jaisnan Aug 22, 2024
3f530a4
add @carolynzech to approvers list for testing
jaisnan Aug 22, 2024
0fd93bf
Revert requirement back to 1
jaisnan Aug 22, 2024
233661d
Change approvers to 2
jaisnan Aug 22, 2024
22e770b
resolve conflict
jaisnan Aug 22, 2024
1f6eceb
merge from main
jaisnan Aug 22, 2024
ff9ff58
Merge branch 'main' into modify-2
jaisnan Aug 22, 2024
5ac231e
remove github createa
jaisnan Aug 22, 2024
257f8bb
Merge branch 'modify-2' of https://github.com/jaisnan/rust-dev into m…
jaisnan Aug 22, 2024
6cac23c
update trigger condition
jaisnan Aug 22, 2024
b079d37
Re-run on pull and approval as well
jaisnan Aug 22, 2024
efc68d0
try with edited pull_request trigger
jaisnan Aug 22, 2024
9557879
add push paths as well
jaisnan Aug 22, 2024
d3c3e51
Merge branch 'main' into modify-2
zhassan-aws Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/pull_requests.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ members = [
"qinheping",
"tautschnig",
"jaisnan",
"patricklam"
"patricklam",
"carolynzech",
]
39 changes: 12 additions & 27 deletions .github/workflows/pr_approval.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
);
Expand All @@ -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));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered the business logic to a script instead of hard coding it in here? I think it is usually much easier to debug and run it locally. For example, you could create a script that takes as an argument a list of logins and the script succeeds if and only if the minimum number of people in the committee has been achieved.

Or the script output the number of people from the committee from that list and you only check here if the number is >= minimum requirement.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for that idea! I shall move the logic to its own script ad use gh command line to try and rerun old failing checks.


if (!tomlData.committee || !Array.isArray(tomlData.committee.members)) {
throw new Error('committee.members is not an array in the TOML file');
Expand Down Expand Up @@ -110,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.`,
Expand All @@ -124,37 +123,23 @@ jobs:
pull_number
});

// Create or update check run
// Update check run
const checkRuns = await github.rest.checks.listForRef({
owner,
repo,
ref: pr.data.head.sha,
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}`);
Expand Down
Loading