Skip to content

Commit 8c2f2d1

Browse files
infinisilinfinisil
authored
Merge pull request #4 from NixOS/improvements
Various improvements: Better testing, draft PRs, testing documentation and more
2 parents 81a8973 + 22afadd commit 8c2f2d1

File tree

6 files changed

+185
-35
lines changed

6 files changed

+185
-35
lines changed

‎.github/CODEOWNERS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* @NixOS/commit-bit-delegation

‎.github/workflows/retire.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ jobs:
3535
with:
3636
token: ${{ steps.app-token.outputs.token }}
3737
- name: Run script
38-
run: scripts/retire.sh NixOS nixpkgs nixpkgs-committers members
38+
# One month plus a bit of leeway
39+
run: scripts/retire.sh NixOS nixpkgs nixpkgs-committers members "yesterday 1 month ago"
3940
env:
4041
GH_TOKEN: ${{ steps.app-token.outputs.token }}
42+
PROD: "1"

‎.github/workflows/sync.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,4 @@ jobs:
4747
branch: create-pull-request/sync
4848
title: Automated sync
4949
body: This is an automated PR to sync the member list in this repository to match the GitHub team members. This PR can be merged without taking any further action.
50-
team-reviewers: commit-bit-delegation
5150

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.tmp

‎scripts/README.md‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Nixpkgs Committer management script testing
2+
3+
The recommended way to test these scripts is to run them in a GitHub test organisation with the right setup.
4+
Since creating your own takes some time, you can just ask @infinisil to get added to his test setup instead,
5+
whose identifiers will be used here.
6+
7+
## Setup
8+
9+
### One-time
10+
11+
- [infinisil-test-org](https://github.com/infinisil-test-org): A GitHub organisation you're part of
12+
- Repositories:
13+
- [infinisil-test-org/empty](https://github.com/infinisil-test-org/empty): An empty one with no activity on it
14+
- [infinisil-test-org/active](https://github.com/infinisil-test-org/active): One where you have some activity
15+
- [infinisil-test-org/nixpkgs-committers](https://github.com/infinisil-test-org/nixpkgs-committers): A fork of the upstream repo
16+
17+
Useful to keep the first two separate for testing, because it's not possible to "undo" activity on a repo.
18+
- [@infinisil-test-org/actors](https://github.com/orgs/infinisil-test-org/teams/actors): A team you're part of, needs write access to the `active` and `nixpkgs-committers` repository
19+
20+
### Per-user
21+
22+
Once you have the above setup (or got @infinisil to add yourself to his), you have to prepare the following:
23+
24+
- Add some activity of yours to the `active` repo.
25+
To cover all code branches it's recommended to create, push to and delete a branch.
26+
You can do this from the web interface.
27+
- Get the GitHub CLI available (`pkgs.github-cli`) and authenticate it using `gh auth login`
28+
29+
## Testing `sync.sh`
30+
31+
This script has no external effects and as such can be easily tested by running:
32+
33+
```bash
34+
scripts/sync.sh infinisil-test-org actors members
35+
```
36+
37+
Check that it synchronises the files in the `members` directory with the team members of the `actors` team.
38+
39+
## `retire.sh`
40+
41+
This script has external effects and as such needs a bit more care when testing.
42+
43+
### Setup (important!)
44+
45+
To avoid other users getting pings, ensure that the `members` directory contains only your own user, then commit and push it for testing:
46+
47+
```bash
48+
me=$(gh api /user --jq .login)
49+
git switch -c "test-$me"
50+
rm -rf members
51+
mkdir members
52+
touch "members/$me"
53+
git add members
54+
git commit -m testing
55+
git push -u [email protected]:infinisil-test-org/nixpkgs-committers HEAD
56+
```
57+
58+
### Test sequence
59+
60+
The following sequence tests all code paths:
61+
62+
1. Run the script with the `active` repo argument to simulate CI running without inactive users:
63+
```bash
64+
scripts/retire.sh infinisil-test-org active nixpkgs-committers members 'yesterday 1 month ago'
65+
```
66+
67+
Check that no PR would be opened.
68+
2. Run the script with the `empty` repo argument to simulate CI running with inactive users:
69+
70+
```bash
71+
scripts/retire.sh infinisil-test-org empty nixpkgs-committers members 'yesterday 1 month ago'
72+
```
73+
74+
Check that it would create a PR before running it again with `PROD=1` to actually do it:
75+
76+
```bash
77+
PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members 'yesterday 1 month ago'
78+
```
79+
80+
Check that it created the PR appropriately.
81+
You can undo this step by closing the PR.
82+
3. Run it again to simulate CI running again later:
83+
```bash
84+
PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members 'yesterday 1 month ago'
85+
```
86+
Check that no other PR is opened.
87+
4. Run it again with `now` as the date to simulate the time interval passing:
88+
```bash
89+
PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members now
90+
```
91+
Check that it undrafted the previous PR and posted an appropriate comment.
92+
5. Run it again to simulate CI running again later:
93+
```bash
94+
PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members now
95+
```
96+
6. Reset by marking the PR as a draft again.
97+
7. Run it again with the `active` repo argument to simulate activity during the time interval:
98+
```bash
99+
PROD=1 scripts/retire.sh infinisil-test-org active nixpkgs-committers members now
100+
```

‎scripts/retire.sh‎

Lines changed: 80 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,89 @@
11
#!/usr/bin/env bash
22
set -euo pipefail
3+
shopt -s nullglob
4+
5+
log() {
6+
echo "$@" >&2
7+
}
8+
9+
trace() {
10+
log "Running:" "${@@Q}"
11+
"$@"
12+
}
13+
14+
effect() {
15+
log -en "\e[33m"
16+
if [[ -z "${PROD:-}" ]]; then
17+
log "Skipping effect:" "${@@Q}"
18+
# If there's stdin, show it
19+
if read -t 0 _; then
20+
sed "s/^/[stdin] /" >&2
21+
fi
22+
else
23+
trace "$@"
24+
fi
25+
log -en "\e[0m"
26+
}
327

428
usage() {
5-
echo >&2 "Usage: $0 ORG ACTIVITY_REPO MEMBER_REPO DIR"
29+
log "Usage: $0 ORG ACTIVITY_REPO MEMBER_REPO DIR PR_CUTOFF_DATE"
630
exit 1
731
}
832

933
ORG=${1:-$(usage)}
1034
ACTIVITY_REPO=${2:-$(usage)}
1135
MEMBER_REPO=${3:-$(usage)}
1236
DIR=${4:-$(usage)}
37+
CUTOFF_DATE=${5:-$(usage)}
1338

14-
tmp=$(mktemp -d)
15-
16-
shopt -s nullglob
17-
18-
# One month plus a bit of leeway
19-
epochOneMonthAgo=$(( $(date --date='1 month ago' +%s) - 60 * 60 * 12 ))
2039
mainBranch=$(git branch --show-current)
40+
cutoffEpoch=$(date --date="$CUTOFF_DATE" +%s)
41+
42+
if [[ -z "${PROD:-}" ]]; then
43+
tmp=$(git rev-parse --show-toplevel)/.tmp
44+
rm -rf "$tmp"
45+
mkdir "$tmp"
46+
log -e "\e[33mPROD=1 is not set, skipping effects and keeping temporary files in $tmp until the next run\e[0m"
47+
else
48+
tmp=$(mktemp -d)
49+
trap 'rm -rf "$tmp"' exit
50+
fi
2151

2252
mkdir -p "$DIR"
2353
cd "$DIR"
2454
for login in *; do
25-
gh api -X GET /repos/"$ORG"/"$ACTIVITY_REPO"/activity -f time_period=year -f actor="$login" -f per_page=100 \
26-
--jq ".[] | \"- \(.timestamp) [\(.activity_type) on \(.ref | ltrimstr(\"refs/heads/\"))](https://github.com/$ORG/$ACTIVITY_REPO/compare/\(.before)...\(.after))\"" \
55+
trace gh api -X GET /repos/"$ORG"/"$ACTIVITY_REPO"/activity \
56+
-f time_period=year \
57+
-f actor="$login" \
58+
-f per_page=100 \
59+
--jq '.[] |
60+
"- \(.timestamp) [\(.activity_type) on \(.ref | ltrimstr("refs/heads/"))](https://github.com/'"$ORG/$ACTIVITY_REPO"'/\(
61+
if .activity_type == "branch_creation" then
62+
"commit/\(.after)"
63+
elif .activity_type == "branch_deletion" then
64+
"commit/\(.before)"
65+
else
66+
"compare/\(.before)...\(.after)"
67+
end
68+
))"' \
2769
> "$tmp/$login"
2870
activityCount=$(wc -l <"$tmp/$login")
2971

3072
branchName=retire-$login
31-
prInfo=$(gh api -X GET /repos/"$ORG"/"$MEMBER_REPO"/pulls -f head="$ORG":"$branchName" --jq '.[0]')
73+
prInfo=$(trace gh api -X GET /repos/"$ORG"/"$MEMBER_REPO"/pulls -f head="$ORG":"$branchName" --jq '.[0]')
3274
if [[ -n "$prInfo" ]]; then
3375
# If there is a PR already
3476
prNumber=$(jq .number <<< "$prInfo")
3577
epochCreatedAt=$(date --date="$(jq -r .created_at <<< "$prInfo")" +%s)
36-
if (( epochCreatedAt < epochOneMonthAgo )); then
37-
echo "$login has a retirement PR due, comment with a reminder to merge"
78+
if jq -e .draft <<< "$prInfo" >/dev/null && (( epochCreatedAt < cutoffEpoch )); then
79+
log "$login has a retirement PR due, unmarking PR as draft and commenting with next steps"
80+
effect gh pr ready --repo "$ORG/$MEMBER_REPO" "$prNumber"
3881
{
3982
if (( activityCount > 0 )); then
4083
echo "One month has passed, @$login has been active again:"
4184
cat "$tmp/$login"
4285
echo ""
43-
echo "This PR may be merged and implemented by:"
86+
echo "If still appropriate, this PR may be merged and implemented by:"
4487
else
4588
echo "One month has passed, to this PR should now be merged and implemented by:"
4689
fi
@@ -57,30 +100,34 @@ for login in *; do
57100
echo ' --method DELETE \'
58101
echo " '/orgs/NixOS/teams/nixpkgs-committers/memberships/$login'"
59102
echo ' ```'
60-
} | gh api --method POST /repos/"$ORG"/"$MEMBER_REPO"/issues/"$prNumber"/comments -F "body=@-" >/dev/null
103+
} | effect gh api --method POST /repos/"$ORG"/"$MEMBER_REPO"/issues/"$prNumber"/comments -F "body=@-" >/dev/null
61104
else
62-
echo "$login has a retirement PR pending"
105+
log "$login has a retirement PR pending"
63106
fi
64107
elif (( activityCount <= 0 )); then
65-
echo "$login has become inactive, opening a PR"
108+
log "$login has become inactive, opening a PR"
66109
# If there is no PR yet, but they have become inactive
67-
git switch -C "$branchName"
68-
git rm "$login"
69-
git commit -m "Automatic retirement of @$login"
70-
git push -f origin "$branchName"
71-
{
72-
echo "This is an automated PR to retire @$login as a Nixpkgs committers due to not using their commit access for 1 year."
73-
echo ""
74-
echo "Make a comment with your motivation to keep commit access, otherwise this PR will be merged and implemented in 1 month."
75-
} | gh api \
76-
--method POST \
77-
/repos/"$ORG"/"$MEMBER_REPO"/pulls \
78-
-f "title=Automatic retirement of @$login" \
79-
-F "body=@-" \
80-
-f "head=$ORG:$branchName" \
81-
-f "base=$mainBranch" >/dev/null
82-
git checkout "$mainBranch"
110+
(
111+
trace git switch -C "$branchName"
112+
trap 'trace git checkout "$mainBranch" && trace git branch -D "$branchName"' exit
113+
trace git rm "$login"
114+
trace git commit -m "Automatic retirement of @$login"
115+
effect git push -f -u [email protected]:"$ORG"/"$MEMBER_REPO" "$branchName"
116+
{
117+
echo "This is an automated PR to retire @$login as a Nixpkgs committers due to not using their commit access for 1 year."
118+
echo ""
119+
echo "Make a comment with your motivation to keep commit access, otherwise this PR will be merged and implemented in 1 month."
120+
} | effect gh api \
121+
--method POST \
122+
/repos/"$ORG"/"$MEMBER_REPO"/pulls \
123+
-f "title=Automatic retirement of @$login" \
124+
-F "body=@-" \
125+
-f "head=$ORG:$branchName" \
126+
-f "base=$mainBranch" \
127+
-F "draft=true" >/dev/null
128+
)
83129
else
84-
echo "$login is active with $activityCount activities"
130+
log "$login is active with $activityCount activities"
85131
fi
132+
log ""
86133
done

0 commit comments

Comments
 (0)