From cf0616fec668a6e759259348e973ef2ff3d547e5 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 4 Jan 2025 13:54:05 +0100 Subject: [PATCH 1/5] ci/request-reviews: rename code-owner related files Now that we have maintainer reviews as well, be a bit more explicit about naming. --- .github/workflows/codeowners-v2.yml | 2 +- ci/request-reviews/default.nix | 4 ++-- ci/request-reviews/{get-reviewers.sh => get-code-owners.sh} | 0 .../{request-reviews.sh => request-code-owner-reviews.sh} | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename ci/request-reviews/{get-reviewers.sh => get-code-owners.sh} (100%) rename ci/request-reviews/{request-reviews.sh => request-code-owner-reviews.sh} (96%) diff --git a/.github/workflows/codeowners-v2.yml b/.github/workflows/codeowners-v2.yml index 0a4f97b1395fb..ae95fd0de05fb 100644 --- a/.github/workflows/codeowners-v2.yml +++ b/.github/workflows/codeowners-v2.yml @@ -106,6 +106,6 @@ jobs: run: nix-build ci -A requestReviews - name: Request reviews - run: result/bin/request-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE" + run: result/bin/request-code-owner-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE" env: GH_TOKEN: ${{ steps.app-token.outputs.token }} diff --git a/ci/request-reviews/default.nix b/ci/request-reviews/default.nix index f5f8dc1deb004..e288fef4f9f61 100644 --- a/ci/request-reviews/default.nix +++ b/ci/request-reviews/default.nix @@ -14,9 +14,9 @@ stdenvNoCC.mkDerivation { src = lib.fileset.toSource { root = ./.; fileset = lib.fileset.unions [ - ./get-reviewers.sh + ./get-code-owners.sh ./process-reviewers.sh - ./request-reviews.sh + ./request-code-owner-reviews.sh ./verify-base-branch.sh ./dev-branches.txt ]; diff --git a/ci/request-reviews/get-reviewers.sh b/ci/request-reviews/get-code-owners.sh similarity index 100% rename from ci/request-reviews/get-reviewers.sh rename to ci/request-reviews/get-code-owners.sh diff --git a/ci/request-reviews/request-reviews.sh b/ci/request-reviews/request-code-owner-reviews.sh similarity index 96% rename from ci/request-reviews/request-reviews.sh rename to ci/request-reviews/request-code-owner-reviews.sh index 986f3684b42a2..27725bd50773f 100755 --- a/ci/request-reviews/request-reviews.sh +++ b/ci/request-reviews/request-code-owner-reviews.sh @@ -78,7 +78,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" "$ownersFile" "$baseBranch" "$headRef" | \ +"$SCRIPT_DIR"/get-code-owners.sh "$tmp/nixpkgs.git" "$ownersFile" "$baseBranch" "$headRef" | \ "$SCRIPT_DIR"/process-reviewers.sh "$baseRepo" "$prNumber" "$prAuthor" > "$tmp/reviewers.json" log "Requesting reviews from: $(<"$tmp/reviewers.json")" From ffb0ace1e759bd2b3ff6a6a6680b3cc1c52cd517 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 4 Jan 2025 14:28:06 +0100 Subject: [PATCH 2/5] ci/request-reviews: use generic wording instead of "code owner" This is now re-used for both code owners and maintainers. --- ci/request-reviews/process-reviewers.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/request-reviews/process-reviewers.sh b/ci/request-reviews/process-reviewers.sh index de969c5c9e4f0..4ca5b8ace7aaa 100755 --- a/ci/request-reviews/process-reviewers.sh +++ b/ci/request-reviews/process-reviewers.sh @@ -42,7 +42,7 @@ gh api \ # 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" + log "User $user is a potential reviewer, but has already left a review, ignoring" unset 'users[${user,,}]' fi done < "$tmp/already-reviewed-by" From 2e6119462e512932cc563e085cb9c4e59156eefd Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 4 Jan 2025 14:08:03 +0100 Subject: [PATCH 3/5] workflows/eval: make "requesting maintainer reviews" separate step Odd to have this in the "Tagging pull request" step, which is only about labels otherwise. --- .github/workflows/eval.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 2e99bb17b8bb8..3c24ec77ae2fb 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -254,7 +254,7 @@ jobs: - name: Build the requestReviews derivation run: nix-build base/ci -A requestReviews - - name: Tagging pull request + - name: Labelling pull request run: | # Get all currently set rebuild labels gh api \ @@ -283,6 +283,13 @@ jobs: -f "labels[]=$toAdd" done < <(comm -13 before after) + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + NUMBER: ${{ github.event.number }} + + - name: Requesting maintainer reviews + run: | # maintainers.json contains GitHub IDs. Look up handles to request reviews from. # There appears to be no API to request reviews based on GitHub IDs jq -r 'keys[]' comparison/maintainers.json \ From 62779fbfa4499ebe9545047980933022e8a11bb0 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 4 Jan 2025 14:05:18 +0100 Subject: [PATCH 4/5] ci/request-reviews: share code to request reviewers from gh api This makes it easier to add ofborg's request-1-by-1 logic, where failed requests are OK for edge cases. --- .github/workflows/eval.yml | 9 +----- ci/request-reviews/default.nix | 2 +- .../request-code-owner-reviews.sh | 18 ++--------- ...cess-reviewers.sh => request-reviewers.sh} | 31 +++++++++++++++++-- 4 files changed, 32 insertions(+), 28 deletions(-) rename ci/request-reviews/{process-reviewers.sh => request-reviewers.sh} (69%) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 3c24ec77ae2fb..72b2103b87207 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -294,14 +294,7 @@ jobs: # There appears to be no API to request reviews based on GitHub IDs jq -r 'keys[]' comparison/maintainers.json \ | while read -r id; do gh api /user/"$id" --jq .login; done \ - | GH_TOKEN=${{ steps.app-token.outputs.token }} result/bin/process-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR" \ - > reviewers.json - - # Request reviewers from maintainers of changed output paths - GH_TOKEN=${{ steps.app-token.outputs.token }} gh api \ - --method POST \ - /repos/"$REPOSITORY"/pulls/"$NUMBER"/requested_reviewers \ - --input reviewers.json + | GH_TOKEN=${{ steps.app-token.outputs.token }} result/bin/request-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR" env: GH_TOKEN: ${{ github.token }} diff --git a/ci/request-reviews/default.nix b/ci/request-reviews/default.nix index e288fef4f9f61..b180d60be97c3 100644 --- a/ci/request-reviews/default.nix +++ b/ci/request-reviews/default.nix @@ -15,7 +15,7 @@ stdenvNoCC.mkDerivation { root = ./.; fileset = lib.fileset.unions [ ./get-code-owners.sh - ./process-reviewers.sh + ./request-reviewers.sh ./request-code-owner-reviews.sh ./verify-base-branch.sh ./dev-branches.txt diff --git a/ci/request-reviews/request-code-owner-reviews.sh b/ci/request-reviews/request-code-owner-reviews.sh index 27725bd50773f..fefc8c3be3fa2 100755 --- a/ci/request-reviews/request-code-owner-reviews.sh +++ b/ci/request-reviews/request-code-owner-reviews.sh @@ -77,20 +77,6 @@ if ! "$SCRIPT_DIR"/verify-base-branch.sh "$tmp/nixpkgs.git" "$headRef" "$baseRep exit 1 fi -log "Getting code owners to request reviews from" +log "Requesting reviews from code owners" "$SCRIPT_DIR"/get-code-owners.sh "$tmp/nixpkgs.git" "$ownersFile" "$baseBranch" "$headRef" | \ - "$SCRIPT_DIR"/process-reviewers.sh "$baseRepo" "$prNumber" "$prAuthor" > "$tmp/reviewers.json" - -log "Requesting reviews from: $(<"$tmp/reviewers.json")" - -if ! response=$(effect gh api \ - --method POST \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \ - --input "$tmp/reviewers.json"); then - log "Failed to request reviews: $response" - exit 1 -fi - -log "Successfully requested reviews" + "$SCRIPT_DIR"/request-reviewers.sh "$baseRepo" "$prNumber" "$prAuthor" diff --git a/ci/request-reviews/process-reviewers.sh b/ci/request-reviews/request-reviewers.sh similarity index 69% rename from ci/request-reviews/process-reviewers.sh rename to ci/request-reviews/request-reviewers.sh index 4ca5b8ace7aaa..1f6311e89c849 100755 --- a/ci/request-reviews/process-reviewers.sh +++ b/ci/request-reviews/request-reviewers.sh @@ -1,15 +1,26 @@ #!/usr/bin/env bash -# Process reviewers for a PR, reading line-separated usernames on stdin, -# returning a JSON suitable to be consumed by the API endpoint to request reviews: +# Request reviewers for a PR, reading line-separated usernames on stdin, +# filtering for valid reviewers before using the API endpoint to request reviews: # https://docs.github.com/en/rest/pulls/review-requests?apiVersion=2022-11-28#request-reviewers-for-a-pull-request set -euo pipefail +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' exit + log() { echo "$@" >&2 } +effect() { + if [[ -n "${DRY_MODE:-}" ]]; then + log "Skipping in dry mode:" "${@@Q}" + else + "$@" + fi +} + if (( "$#" < 3 )); then log "Usage: $0 BASE_REPO PR_NUMBER PR_AUTHOR" exit 1 @@ -62,4 +73,18 @@ jq -n \ --arg users "${!users[*]}" \ '{ reviewers: $users | split(" "), - }' + }' > "$tmp/reviewers.json" + +log "Requesting reviews from: $(<"$reviewersFile")" + +if ! response=$(effect gh api \ + --method POST \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \ + --input "$tmp/reviewers.json"); then + log "Failed to request reviews: $response" + exit 1 +fi + +log "Successfully requested reviews" From 034613f860fcd339bd2c20c8f6bc259a2f9dc034 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 4 Jan 2025 14:39:15 +0100 Subject: [PATCH 5/5] ci/request-reviews: request reviewers 1-by-1 This is to be able to ignore the odd failure for some users, who are listed as collaborators, but still fail to be requested properly. --- ci/request-reviews/request-reviewers.sh | 33 ++++++++++--------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/ci/request-reviews/request-reviewers.sh b/ci/request-reviews/request-reviewers.sh index 1f6311e89c849..7bb4d110fe1bd 100755 --- a/ci/request-reviews/request-reviewers.sh +++ b/ci/request-reviews/request-reviewers.sh @@ -68,23 +68,16 @@ for user in "${!users[@]}"; do fi done -# Turn it into a JSON for the GitHub API call to request PR reviewers -jq -n \ - --arg users "${!users[*]}" \ - '{ - reviewers: $users | split(" "), - }' > "$tmp/reviewers.json" - -log "Requesting reviews from: $(<"$reviewersFile")" - -if ! response=$(effect gh api \ - --method POST \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \ - --input "$tmp/reviewers.json"); then - log "Failed to request reviews: $response" - exit 1 -fi - -log "Successfully requested reviews" +for user in "${!users[@]}"; do + log "Requesting review from: $user" + + if ! response=$(jq -n --arg user "$user" '{ reviewers: [ $user ] }' | \ + effect gh api \ + --method POST \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + "/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \ + --input -); then + log "Failed to request review from $user: $response" + fi +done