Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved regular reviews #16

Merged
merged 3 commits into from
Jul 25, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
55 changes: 43 additions & 12 deletions scripts/review-body.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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"