Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 52 additions & 14 deletions ci/request-reviews/get-reviewers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ log() {
echo "$@" >&2
}

if (( "$#" < 5 )); then
log "Usage: $0 GIT_REPO BASE_REF HEAD_REF OWNERS_FILE PR_AUTHOR"
if (( "$#" < 7 )); then
log "Usage: $0 GIT_REPO OWNERS_FILE BASE_REPO BASE_REF HEAD_REF PR_NUMBER PR_AUTHOR"
exit 1
fi

gitRepo=$1
baseRef=$2
headRef=$3
ownersFile=$4
prAuthor=$5
ownersFile=$2
baseRepo=$3
baseRef=$4
headRef=$5
prNumber=$6
prAuthor=$7

tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' exit
Expand All @@ -32,8 +34,8 @@ log "This PR touches ${#touchedFiles[@]} files"
# remove code owners to avoid pinging them
git -C "$gitRepo" show "$baseRef":"$ownersFile" > "$tmp"/codeowners

# Associative arrays with the team/user as the key for easy deduplication
declare -A teams users
# Associative array with the user as the key for easy de-duplication
declare -A users=()

for file in "${touchedFiles[@]}"; do
result=$(codeowners --file "$tmp"/codeowners "$file")
Expand All @@ -59,10 +61,34 @@ for file in "${touchedFiles[@]}"; do
fi
# The first regex match is everything after the @
entry=${BASH_REMATCH[1]}
if [[ "$entry" =~ .*/(.*) ]]; then
# Teams look like $org/$team, where we only need $team for the API
# call to request reviews from teams
teams[${BASH_REMATCH[1]}]=

if [[ "$entry" =~ (.*)/(.*) ]]; then
# Teams look like $org/$team
org=${BASH_REMATCH[1]}
team=${BASH_REMATCH[2]}

# Instead of requesting a review from the team itself,
# we request reviews from the individual users.
# This is because once somebody from a team reviewed the PR,
# the API doesn't expose that the team was already requested for a review,
# so we wouldn't be able to avoid rerequesting reviews
# without saving some some extra state somewhere

# We could also consider implementing a more advanced heuristic
# in the future that e.g. only pings one team member,
# but escalates to somebody else if that member doesn't respond in time.
gh api \
--cache=1h \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"/orgs/$org/teams/$team/members" \
--jq '.[].login' > "$tmp/team-members"
readarray -t members < "$tmp/team-members"
log "Team $entry has these members: ${members[*]}"

for user in "${members[@]}"; do
users[$user]=
done
else
# Everything else is a user
users[$entry]=
Expand All @@ -77,11 +103,23 @@ if [[ -v users[$prAuthor] ]]; then
unset 'users[$prAuthor]'
fi

gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"/repos/$baseRepo/pulls/$prNumber/reviews" \
--jq '.[].user.login' > "$tmp/already-reviewed-by"

# And we don't want to rerequest reviews from people who already reviewed
while read -r user; do
if [[ -v users[$user] ]]; then
log "User $user is a code owner but has already left a review, ignoring"
unset 'users[$user]'
fi
done < "$tmp/already-reviewed-by"

# Turn it into a JSON for the GitHub API call to request PR reviewers
jq -n \
--arg users "${!users[*]}" \
--arg teams "${!teams[*]}" \
'{
reviewers: $users | split(" "),
team_reviewers: $teams | split(" ")
}'
2 changes: 1 addition & 1 deletion ci/request-reviews/request-reviews.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ if ! "$SCRIPT_DIR"/verify-base-branch.sh "$tmp/nixpkgs.git" "$headRef" "$baseRep
fi

log "Getting code owners to request reviews from"
"$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile" "$prAuthor" > "$tmp/reviewers.json"
"$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$ownersFile" "$baseRepo" "$baseBranch" "$headRef" "$prNumber" "$prAuthor" > "$tmp/reviewers.json"

log "Requesting reviews from: $(<"$tmp/reviewers.json")"

Expand Down