Skip to content

Commit

Permalink
Jm/codeowners revamp (#617)
Browse files Browse the repository at this point in the history
### Description

- Changes the CODEOWNERS to have +1 and +2 reviewers.
- Adds a workflow to validate that a +2 reviewer has approved the code
change.
- Adds some early +2 reviewers
- TODO: Change the contributing guidelines to reflect the changes made
herein.

### Type of changes
<!-- Mark the relevant option with an [x] -->

- [ ]  Bug fix (non-breaking change which fixes an issue)
- [X]  New feature (non-breaking change which adds functionality)
- [ ]  Refactor
- [X]  Documentation update
- [ ]  Other (please describe):

### CI Pipeline Configuration
Configure CI behavior by applying the relevant labels:

-
[SKIP_CI](https://github.com/NVIDIA/bionemo-framework/blob/main/docs/docs/user-guide/contributing/contributing.md#skip_ci)
- Skip all continuous integration tests
-
[INCLUDE_NOTEBOOKS_TESTS](https://github.com/NVIDIA/bionemo-framework/blob/main/docs/docs/user-guide/contributing/contributing.md#include_notebooks_tests)
- Execute notebook validation tests in pytest

> [!NOTE]
> By default, the notebooks validation tests are skipped unless
explicitly enabled.

### Usage
<!--- How does a user interact with the changed code -->
```python
TODO: Add code snippet
```

### Pre-submit Checklist
<!--- Ensure all items are completed before submitting -->

 - [ ] I have tested these changes locally
 - [ ] I have updated the documentation accordingly
 - [ ] I have added/updated tests as needed
 - [ ] All existing tests pass successfully

Signed-off-by: Jonathan <jomitchell@nvidia.com>
Co-authored-by: Jonathan <jomitchell>
Signed-off-by: Polina Binder <pbinder@nvidia.com>
  • Loading branch information
jomitchellnv authored and polinabinder1 committed Jan 22, 2025
1 parent a03637e commit 15ad167
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 6 deletions.
189 changes: 189 additions & 0 deletions .github/workflows/approvals.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
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.
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
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const { data: reviews } = await github.rest.pulls.listReviews({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.payload.pull_request.number,
});
const latestReviews = {};
for (const review of reviews) {
latestReviews[review.user.login] = review.state;
}
console.log('Latest Reviews:', latestReviews);
const approvedUsers = Object.keys(latestReviews).filter(user => latestReviews[user] === 'APPROVED');
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"
# 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
TIER2_APPROVED=true
break 2
fi
done
done
if [[ "$TIER2_APPROVED" == "true" ]]; then
echo "tier2Approved=true" >> $GITHUB_ENV
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."
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."
17 changes: 11 additions & 6 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@
# @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

# TIER 1 Approvers below.
#
## LEGAL
#
Expand All @@ -42,13 +47,13 @@ license_header @dorotat-nv @jstjohn @malcolmgreaves @trvachov
#
## DOCUMENTATION
#
README.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn
docs @dorotat-nv @jstjohn @malcolmgreaves @pstjohn
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
CONTRIBUTING.md @jstjohn @malcolmgreaves @pstjohn @trvachov
docs/CODE-REVIEW.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
docs/CONTRIBUTING.md @dorotat-nv @jstjohn @malcolmgreaves @pstjohn @trvachov
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


#
Expand Down

0 comments on commit 15ad167

Please sign in to comment.