diff --git a/ci/request-reviews/get-reviewers.sh b/ci/request-reviews/get-reviewers.sh index d5cd9e0a4fd4a..be0fd10c5b22f 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 @@ -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") @@ -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]= @@ -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(" ") }' diff --git a/ci/request-reviews/request-reviews.sh b/ci/request-reviews/request-reviews.sh index d62ab309bcc8c..a70a95a65b3ac 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")"