Removes tier1 approval rules, sets triggers pull_request_reviews #629
Conversation
jomitchellnv
requested review from
jstjohn,
pstjohn and
trvachov
as code owners
There was a problem hiding this comment.
Instead of having additional fiels, why not to use group tags for github? https://github.com/NVIDIA/Megatron-LM/blob/4fb4c3dc61b26784ccf804b340ba047bbcec3953/CODEOWNERS#L7
way easier to manage groups and their permissions and scopes
.github/workflows/approvals.yml
Outdated
| - 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 | ||
|
|
||
| done < CODEOWNERS | ||
|
|
||
| # Export TIER2 Reviewers as JSON | ||
| echo "$(declare -p TIER2_REVIEWERS)" > tier2reviewers.json | ||
| echo "TIER2 reviewers exported to JSON." |
There was a problem hiding this comment.
Can we just have the list of +2 approvers live in this workflow file? That way it's separated from the CODEOWNERS file, and we don't have to do this parsing step or the checkout step which saves us some CI time.
There was a problem hiding this comment.
I would prefer to have a user group for that and use it here
There was a problem hiding this comment.
ie Add an approval rule section in https://github.com/gitlabhq/gitlabhq/blob/master/doc/user/project/merge_requests/approvals/rules.md
There was a problem hiding this comment.
There was a problem hiding this comment.
I think its hidden if its inside this workflow and not available in the CODEOWNERS file.
.github/workflows/approvals.yml
Outdated
| # Strip '@' from REVIEWER | ||
| CLEAN_REVIEWER="${REVIEWER#@}" |
There was a problem hiding this comment.
could probably remove if we just have a list of +2 folks in this action
.github/workflows/approvals.yml
Outdated
| - name: Enforce approval requirements | ||
| run: | | ||
| echo "Enforcing approval requirements..." | ||
|
|
||
| if [[ "$tier2Approved" != "true" ]]; then | ||
| echo "ERROR: No +2 reviewer has approved the pull request." | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "All tiered 2 approval requirements met. Proceeding." |
There was a problem hiding this comment.
Can we just merge this with the previous step?
Signed-off-by: Jonathan <[email protected]>
jomitchellnv
force-pushed
the
jm/approvals-wf-rules
branch
from
2928a25 to
e0d1620
Compare
Signed-off-by: Jonathan <[email protected]>
|
closing in favor of #639 |
Description
Type of changes
CI Pipeline Configuration
Configure CI behavior by applying the relevant labels:
Note
By default, the notebooks validation tests are skipped unless explicitly enabled.
Usage
Pre-submit Checklist