Skip to content

Commit

Permalink
New approvals workflow (#639)
Browse files Browse the repository at this point in the history 
Updates the approvals workflow to only check for +2 reviews and only run
on PR reviews.

+1 reviews will be enforced via github's CODEOWNERS check

---------

Signed-off-by: Jonathan <[email protected]>
Signed-off-by: Peter St. John <[email protected]>
Co-authored-by: Jonathan <[email protected]>
  • Loading branch information
@pstjohn @jomitchellnv
pstjohn and jomitchellnv authored Jan 22, 2025
1 parent 14c0f14 commit 911a71a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 163 deletions.
143 changes: 10 additions & 133 deletions .github/workflows/approvals.yml
View file
Original file line number Diff line number Diff line change
@@ -1,76 +1,16 @@
name: Enforce Tiered Approvals

on:
pull_request:
types:
- review_requested
- review_submitted
- synchronize
- opened
- closed
- edited
- reopened
pull_request_review:
types:
- submitted
- edited
- dismissed

# TODO: Action should run when someone approves / disapproves etc.
env:
TIER2_REVIEWERS: "jstjohn,trvachov,pstjohn"

jobs:
enforce_tiered_approvals:
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v3

- name: Parse CODEOWNERS file
id: parse_codeowners
run: |
echo "Parsing CODEOWNERS file..."
declare -A CODEOWNERS
declare -A TIER2_REVIEWERS
while IFS= read -r line; do
# Skip comments and empty lines
[[ "$line" =~ ^#.*$ || -z "$line" ]] && continue
# Detect tier2 reviewers
if [[ "$line" =~ "# tier2" ]]; then
reviewers=$(echo "$line" | awk '{$1=""; $NF=""; print $0}' | xargs)
for reviewer in $reviewers; do
TIER2_REVIEWERS["$reviewer"]=1
done
continue
fi
# Parse +1 CODEOWNERS
path=$(echo "$line" | awk '{print $1}')
owners=$(echo "$line" | awk '{$1=""; print $0}' | xargs)
CODEOWNERS["$path"]="$owners"
done < CODEOWNERS
# Export mappings as JSON
echo "$(declare -p CODEOWNERS)" > codeowners.json
echo "$(declare -p TIER2_REVIEWERS)" > tier2reviewers.json
echo "CODEOWNERS and TIER2 reviewers exported to JSON."
- name: Get changed files
id: get_files
uses: actions/github-script@v6
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const { data: files } = await github.rest.pulls.listFiles({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.payload.pull_request.number,
});
const changedFiles = files.map(file => file.filename);
core.setOutput('changedFiles', changedFiles.join(','));
- name: Get PR reviews
id: get_reviews
uses: actions/github-script@v6
Expand All @@ -94,96 +34,33 @@ jobs:
core.setOutput('approvedUsers', approvedUsers.join(','));
- name: Check +1 approvals (file-specific)
id: check_tier1
run: |
echo "Checking for +1 approvals for changed files..."
CHANGED_FILES="${{ steps.get_files.outputs.changedFiles }}"
APPROVED_USERS="${{ steps.get_reviews.outputs.approvedUsers }}"
# Load CODEOWNERS mapping
declare -A CODEOWNERS
eval "$(cat codeowners.json)"
TIER1_APPROVED=false
# Loop through changed files and verify approval
IFS=',' read -ra FILES <<< "$CHANGED_FILES"
IFS=',' read -ra USERS <<< "$APPROVED_USERS"
for FILE in "${FILES[@]}"; do
for PATTERN in "${!CODEOWNERS[@]}"; do
if [[ "$FILE" == $PATTERN* ]]; then
for OWNER in ${CODEOWNERS[$PATTERN]}; do
# Strip '@' from OWNER
CLEAN_OWNER="${OWNER#@}"
echo "Comparing APPROVED_USERS with CLEAN_OWNER: $CLEAN_OWNER"
if [[ " ${USERS[@]} " =~ " $CLEAN_OWNER " ]]; then
TIER1_APPROVED=true
break 3
fi
done
fi
done
done
if [[ "$TIER1_APPROVED" == "true" ]]; then
echo "tier1Approved=true" >> $GITHUB_ENV
else
echo "tier1Approved=false" >> $GITHUB_ENV
fi
echo $TIER1_APPROVED
- name: Check +2 approvals (global tier)
id: check_tier2
run: |
echo "Checking for +2 approvals..."
APPROVED_USERS="${{ steps.get_reviews.outputs.approvedUsers }}"
# Load TIER2_REVIEWERS mapping
declare -A TIER2_REVIEWERS
eval "$(cat tier2reviewers.json)"
TIER2_APPROVED=false
echo "Approved Users: $APPROVED_USERS"
echo "Tier 2 Reviewers: $TIER2_REVIEWERS"
IFS=',' read -ra reviewer_array <<< "$TIER2_REVIEWERS"
# Iterate over approved users and compare with cleaned TIER2_REVIEWERS
for USER in ${APPROVED_USERS//,/ }; do
echo "Checking approved USER: $USER"
echo "TIER2_REVIEWERS: ${!TIER2_REVIEWERS[@]}"
for REVIEWER in "${!TIER2_REVIEWERS[@]}"; do
# Strip '@' from REVIEWER
CLEAN_REVIEWER="${REVIEWER#@}"
echo "Comparing USER: $USER with CLEAN_REVIEWER: $CLEAN_REVIEWER"
if [[ "$USER" == "$CLEAN_REVIEWER" ]]; then
for REVIEWER in "${reviewer_array[@]}"; do
echo "Comparing USER: $USER with REVIEWER: $REVIEWER"
if [[ "$USER" == "$REVIEWER" ]]; then
TIER2_APPROVED=true
break 2
fi
done
done
if [[ "$TIER2_APPROVED" == "true" ]]; then
echo "tier2Approved=true" >> $GITHUB_ENV
echo "A +2 reviewer has approved the pull request."
else
echo "tier2Approved=false" >> $GITHUB_ENV
fi
echo "TIER2_APPROVED: $TIER2_APPROVED"
- name: Enforce approval requirements
run: |
echo "Enforcing approval requirements..."
if [[ "$tier1Approved" != "true" ]]; then
echo "ERROR: No +1 reviewer has approved the pull request for changed files."
echo "No +2 reviewer has approved the pull request."
exit 1
fi
if [[ "$tier2Approved" != "true" ]]; then
echo "ERROR: No +2 reviewer has approved the pull request."
exit 1
fi
echo "All tiered approval requirements met. Proceeding."
39 changes: 9 additions & 30 deletions CODEOWNERS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,9 @@
# @trvachov - Timur Rvachov
# @yzhang123 - Yang Zhang

# TIER 2 Approvers do not modify the list below.
# Note: the # tier2 comment is a sentinel used to track who is tier2.
# +2 Reviewers (Global Tier)
* @jstjohn @pstjohn @trvachov # tier2
# TODO: make this a team of bionemo-core contributors
* @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov @sichu2023 @skothenhill-nv @jomitchellnv @jwilber @cspades

# TIER 1 Approvers below.
#
## LEGAL
#
Expand All @@ -47,41 +44,23 @@ license_header @dorotat-nv @jstjohn @malcolmgreaves @trvachov
#
## DOCUMENTATION
#
README.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @jwilber
docs @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @jwilber
# These 2 are symlinks: actual content is under docs/
CODE-REVIEW.md @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber
CONTRIBUTING.md @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber
docs/CODE-REVIEW.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber
docs/CONTRIBUTING.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber
**.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber
docs @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov @jwilber


#
## INFRASTRUCTURE
#
.gitignore @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
.dockerignore @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
.devcontainer @dorotat-nv @malcolmgreaves @pstjohn
CODEOWNERS @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
.* @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
*.txt @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
CODEOWNERS @jstjohn @pstjohn @trvachov
Dockerfile @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
justfile @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
internal/ @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
3rdparty @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
pyproject.toml @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
requirements-cve.txt @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
requirements-dev.txt @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
requirements-test.txt @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
tach.yml @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
.secrets.baseline @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
ci/ @dorotat-nv @malcolmgreaves @pstjohn
.nspect-allowlist.toml @dorotat-nv @jstjohn @malcolmgreaves @ohadmo @pstjohn @trvachov
VERSION @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
.github @dorotat-nv @jstjohn @pstjohn @trvachov

#
## LIBRARY CODE
#
scripts @jstjohn @malcolmgreaves @skothenhill-nv

sub-packages/bionemo-fw @DejunL @dorotat-nv @farhadrgh @guoqing-zhou @jstjohn @malcolmgreaves @pstjohn @skothenhill-nv

sub-packages/bionemo-testing @dorotat-nv @farhadrgh @jstjohn @malcolmgreaves @pstjohn @skothenhill-nv
Expand All @@ -92,7 +71,7 @@ sub-packages/bionemo-llm @farhadrgh @dorotat-nv @jstjohn @malcolmgreaves @pstjoh

sub-packages/bionemo-geometric @DejunL @guoqing-zhou @jstjohn @malcolmgreaves

sub-packages/bionemo-esm2 @pstjohn @jstjohn @skothenhill-nv @jomitchell @farhadrgh @simonchu
sub-packages/bionemo-esm2 @pstjohn @jstjohn @skothenhill-nv @jomitchellnv @farhadrgh @sichu2023

sub-packages/bionemo-example_model @jstjohn @malcolmgreaves @skothenhill-nv

Expand Down

0 comments on commit 911a71a

Please sign in to comment.