Merge pull request #16 from NixOS/improved-reviews

Improved regular reviews

.github/workflows/ci.yml

@@ -45,6 +45,7 @@ jobs:
       env:
         # See https://github.com/mszostok/codeowners-validator/blob/main/docs/gh-auth.md#public-repositories
         # And https://github.com/mszostok/codeowners-validator/pull/222#issuecomment-2079521185
+        # The same App is also used in ./review.yml
         GITHUB_APP_ID: ${{ secrets.APP_ID }}
         GITHUB_APP_INSTALLATION_ID: ${{ secrets.APP_INSTALLATION_ID }}
         GITHUB_APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }}

.github/workflows/review.yml

@@ -17,11 +17,18 @@ jobs:
         with:
           path: repo
 
+      - uses: actions/create-github-app-token@v1
+        id: app-token
+        with:
+          # This GitHub App needs Organization/Members/read-only access
+          # to figure out who's part of the team
+          app-id: ${{ secrets.APP_ID }}
+          private-key: ${{ secrets.APP_PRIVATE_KEY }}
+
       - name: Generate issue body
         run: repo/scripts/review-body.sh repo ${{ github.repository }} > body
         env:
-          # This token has read-only admin access to see who has write access to this repo
-          GH_TOKEN: "${{ secrets.OWNERS_VALIDATOR_GITHUB_SECRET }}"
+          GH_TOKEN: ${{ steps.app-token.outputs.token }}
 
       - run: |
           gh api \

CONTRIBUTING.md

@@ -8,12 +8,13 @@ Thus, all organisational changes tracked in this repository should be proposed w
 and the changes should only to be implemented when the PR is merged.
 This may be done manually (e.g. by the person merging the PR) or automatically (e.g. using CD).
 
-All files should have at least somebody in charge of keeping it up-to-date, which should be described with an entry in [CODEOWNERS](./.github/CODEOWNERS). Those people will be requested for a review and be given write access to the repository, see also [permissions of this repository](./doc/org-repo.md).
+All files should have at least somebody in charge of keeping it up-to-date, which should be described with an entry in [CODEOWNERS](./.github/CODEOWNERS).
+Those people will be requested for a review and be given write access to the repository, see also [permissions of this repository](./doc/org-repo.md).
 
 ## Regular manual reviews
 
 Unavoidibly it can also happen for reality to deviate from the documentation without a PR.
-To mitigate this, all people with [code owner entries](./.github/CODEOWNERS) must regularly review their files.
+To mitigate this, all people with [code owner entries](./.github/CODEOWNERS) for files in the `doc` directory must regularly review their files.
 This is done by [automatically opening an issue every month](./.github/workflows/review.yml) to ping all code owners.
 
 This serves as an initial fallback, but more automatic approaches could be implemented in the future, e.g. by scraping and comparing the state.

scripts/review-body.sh

@@ -1,12 +1,10 @@
 #!/usr/bin/env nix-shell
-#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p codeowners github-cli gitMinimal
+#!nix-shell -i bash --pure --keep GH_TOKEN -I nixpkgs=channel:nixpkgs-unstable -p jq codeowners github-cli gitMinimal
 
 set -euo pipefail
 
 # This script outputs the contents of the regular review issue, see ./github/workflows/review.yml
 
-SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-
 if (( $# != 2 )); then
   echo "Usage: $0 PATH OWNER/REPO"
   exit 1
@@ -38,9 +36,49 @@ These are the [current code owners](https://github.com/$repo/tree/$rev/.github/C
 
 declare -A codeowners
 
+cacheDir=${XDG_CACHE_HOME:-$HOME/.cache}/org-review-body
+
+# Resolves a team from e.g. "@NixOS/org" to "@NixOS/foo (@bar @baz)", caching the result
+resolve_team() {
+  local org=$1
+  local id=$2
+  local resolved
+  local cacheFile="$cacheDir/$org/$id"
+  if [[ -f "$cacheFile" ]]; then
+    cat "$cacheFile"
+  else
+    mkdir -p "$(dirname "$cacheFile")"
+    # For a GitHub App, this needs Organization/Members/read-only access
+    resolved=$(gh api \
+      -H "Accept: application/vnd.github+json" \
+      -H "X-GitHub-Api-Version: 2022-11-28" \
+      /orgs/"$org"/teams/"$id"/members |
+      jq -r 'map(.login | "@\(.)") | join(" ")')
+    echo "@$org/$id ($resolved)" | tee "$cacheFile"
+  fi
+}
+
+# Resolves all owners, e.g. from "@foo @NixOS/bar" to "@foo @NixOS/bar (@baz @qux)"
+resolve_owners() {
+  local team=$1
+  local -a entries
+  local result=()
+  IFS=" " read -r -a entries <<< "$team"
+
+  for entry in "${entries[@]}"; do
+    if [[ "$entry" =~ @(.*)/(.*) ]]; then
+      result+=("$(resolve_team "${BASH_REMATCH[1]}" "${BASH_REMATCH[2]}")")
+    else
+      result+=("$entry")
+    fi
+  done
+  echo "${result[*]}"
+}
+
 while read -r file owners; do
   if [[ "$owners" != "(unowned)" ]]; then
-    codeowners[$file]=$owners
+    resolved=$(resolve_owners "$owners")
+    codeowners[$file]=$resolved
   fi
 done < <(cd "$root"; codeowners)
 
@@ -74,11 +112,4 @@ listDir() {
   done
 }
 
-listDir "" ""
-
-echo ""
-
-# Check that all code owners have write permissions
-# `|| true` because this script fails when there are code owners without permissions,
-# which is useful to fail PRs, but not here
-bash "$SCRIPT_DIR"/unprivileged-owners.sh "$root" "$repo" || true
+listDir "" "doc"