From 9a054bb9ddf6413ff5252904321a14b5f1afec9d Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Wed, 9 Oct 2024 22:08:15 +0200
Subject: [PATCH 1/2] ci/request-reviews: Don't rerequest users that already
 reviewed

The automation should never rerequest reviews from users that already
reviewed the changes, which is what was happening before this change:

https://github.com/NixOS/nixpkgs/pull/347354#event-14570645380

Also reorder the arguments to make more sense
---
 ci/request-reviews/get-reviewers.sh   | 28 +++++++++++++++++++++------
 ci/request-reviews/request-reviews.sh |  2 +-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/ci/request-reviews/get-reviewers.sh b/ci/request-reviews/get-reviewers.sh
index d5cd9e0a4fd4..6a7b8edd8202 100755
--- a/ci/request-reviews/get-reviewers.sh
+++ b/ci/request-reviews/get-reviewers.sh
@@ -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
@@ -77,6 +79,20 @@ 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[*]}" \
diff --git a/ci/request-reviews/request-reviews.sh b/ci/request-reviews/request-reviews.sh
index d62ab309bcc8..a70a95a65b3a 100755
--- a/ci/request-reviews/request-reviews.sh
+++ b/ci/request-reviews/request-reviews.sh
@@ -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")"
 

From 1ff83b2c963544c1881f1c5f95fbd09a1ab02a10 Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Wed, 9 Oct 2024 22:12:12 +0200
Subject: [PATCH 2/2] ci/request-reviews: Request reviews for individual team
 members

This makes this codeowner mechanism behave differently than the native
one, but there's no other way to avoid rerequesting reviews from teams
when a member already reviewed the PR.
---
 ci/request-reviews/get-reviewers.sh | 38 +++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/ci/request-reviews/get-reviewers.sh b/ci/request-reviews/get-reviewers.sh
index 6a7b8edd8202..be0fd10c5b22 100755
--- a/ci/request-reviews/get-reviewers.sh
+++ b/ci/request-reviews/get-reviewers.sh
@@ -34,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")
@@ -61,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]=
@@ -96,8 +120,6 @@ 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(" ")
     }'