-
Notifications
You must be signed in to change notification settings - Fork 0
ci: add PR hygiene — UNREVIEWED label + Slack review reminders #12
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
base: master
Are you sure you want to change the base?
Changes from all commits
60cd7c4
186e11e
40af7fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Dogfooding: PRs in this repo get the UNREVIEWED label too. | ||
| # Other repos use templates/pr-label.yml (pinned @master); this one references | ||
| # the reusable workflow relatively so it works on unmerged branches as well. | ||
| name: PR label | ||
| on: | ||
| pull_request: | ||
| types: [opened, reopened] | ||
| permissions: | ||
| pull-requests: write | ||
| issues: write | ||
| jobs: | ||
| label: | ||
| uses: ./.github/workflows/reusable-pr-label.yml | ||
|
severindemchuk marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| name: PR review reminders | ||
|
|
||
| # Monday-morning cron: DMs assignees/reviewers/authors on Slack about open or | ||
| # merged PRs that still carry the UNREVIEWED label. See scripts/pr_reminder.py | ||
| # and the "PR hygiene" section in the README. | ||
|
|
||
| on: | ||
| schedule: | ||
| - cron: '17 6 * * 1' # ~08:17 Zurich in summer, ~07:17 in winter | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read # actions/checkout | ||
| actions: write # keepalive step re-enables this workflow | ||
|
severindemchuk marked this conversation as resolved.
|
||
|
|
||
| jobs: | ||
| remind: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Send reminders | ||
| env: | ||
| # Test phase: fine-grained PAT. Production: replace with a GitHub App | ||
| # token via actions/create-github-app-token (see README) — only this | ||
| # env line changes. | ||
| GH_TOKEN: ${{ secrets.PR_BOT_PAT }} | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} | ||
| GH_SLACK_MAP: ${{ vars.GH_SLACK_MAP }} | ||
| DRY_RUN: ${{ vars.PR_HYGIENE_DRY_RUN }} | ||
| ALLOWLIST: ${{ vars.PR_HYGIENE_ALLOWLIST }} | ||
| run: python3 scripts/pr_reminder.py | ||
|
|
||
| - name: Notify failure on Slack | ||
| if: failure() | ||
| continue-on-error: true | ||
|
severindemchuk marked this conversation as resolved.
|
||
| uses: dreipol/github-actions/slack@master | ||
| env: | ||
| SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} | ||
| TEXT: ':boom: PR review reminder run failed. <https://github.com/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}| Check the logs on github>' | ||
|
severindemchuk marked this conversation as resolved.
|
||
|
|
||
| - name: Keepalive (public repo crons are disabled after 60 idle days) | ||
| if: always() | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: gh api -X PUT "repos/${{ github.repository }}/actions/workflows/pr-reminder.yml/enable" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| name: Reusable PR label | ||
|
|
||
| # Applies the UNREVIEWED label to a pull request. | ||
| # The label is removed MANUALLY by the reviewer after reviewing — that removal | ||
| # is the "reviewed" acknowledgment. This workflow never removes or re-adds it. | ||
| # | ||
| # Consumers call this from a small workflow file, see templates/pr-label.yml. | ||
| # Required caller permissions: pull-requests: write, issues: write | ||
|
|
||
| on: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| label: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Ensure UNREVIEWED label exists | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh label create UNREVIEWED \ | ||
| --color B22D47 \ | ||
| --description "No review yet — remove this label after reviewing" \ | ||
| --force \ | ||
| --repo "${{ github.repository }}" | ||
|
|
||
| - name: Add UNREVIEWED label to PR | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/labels" \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warum ist das
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -> GitHub's REST API has no PR-specific labels endpoint — labels only live under /issues/{number}/labels since a PR is an issue under the hood (gh pr edit --add-label hits the same endpoint internally). So issues here is required, not a mix-up. |
||
| -f "labels[]=UNREVIEWED" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Werden so alle bestehenden Labels überschrieben? Oder ist das ein append?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ist ein append, kein replace |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Slack app manifest for the PR review reminder bot. | ||
| # A Slack workspace admin creates the app at https://api.slack.com/apps | ||
| # ("Create New App" -> "From an app manifest"), installs it to the workspace, | ||
| # and stores the resulting bot token (xoxb-...) as the SLACK_BOT_TOKEN secret | ||
| # in dreipol/github-actions. | ||
| display_information: | ||
| name: PR Review Reminder | ||
| description: DMs you about pull requests waiting for your review | ||
| background_color: "#B22D47" | ||
| features: | ||
| bot_user: | ||
| display_name: pr-review-reminder | ||
| always_online: false | ||
| oauth_config: | ||
| scopes: | ||
| bot: | ||
| - chat:write | ||
| - im:write | ||
| settings: | ||
| org_deploy_enabled: false | ||
| socket_mode_enabled: false | ||
| token_rotation_enabled: false |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,203 @@ | ||
| #!/usr/bin/env python3 | ||
| """Slack reminders for PRs carrying the UNREVIEWED label. | ||
|
|
||
| Searches all open or merged PRs in the org labeled UNREVIEWED (merged PRs | ||
| still nag — e.g. a hotfix reviewed after the fact), and DMs the assignees if | ||
| any, else the requested reviewers, else the author, on Slack. | ||
|
|
||
| Cadence: weekly, Monday mornings, no cap — this is a "don't forget it | ||
| entirely" nudge, not an urgency escalation. Stateless — reminder text shows | ||
| how many weekdays the PR has been unreviewed, derived from the timestamp of | ||
| the (latest) UNREVIEWED "labeled" event on the PR. Removing the label (done | ||
| manually by the reviewer) stops the reminders. | ||
|
|
||
| Environment: | ||
| GH_TOKEN GitHub token with org-wide PR read access (PAT or App token) | ||
| SLACK_BOT_TOKEN Slack bot token (scopes: chat:write, im:write) | ||
| GH_SLACK_MAP JSON object {"github_login": "U_SLACK_MEMBER_ID", ...} | ||
| DRY_RUN "false" enables real Slack sends; anything else = dry run | ||
| ALLOWLIST optional JSON array of GitHub logins; if non-empty, only | ||
| these users receive DMs (pilot phase) | ||
| GH_ORG organization to search (default: dreipol) | ||
| """ | ||
|
|
||
| import json | ||
| import os | ||
| import sys | ||
| import time | ||
| import urllib.parse | ||
| import urllib.request | ||
| from datetime import date, datetime, timezone | ||
| from zoneinfo import ZoneInfo | ||
|
|
||
| GITHUB_API = "https://api.github.com" | ||
| SLACK_API = "https://slack.com/api/chat.postMessage" | ||
| LABEL = "UNREVIEWED" | ||
| LOCAL_TZ = ZoneInfo("Europe/Zurich") | ||
| REQUEST_TIMEOUT = 15 # seconds; a hung connection must not stall the whole run | ||
|
|
||
|
|
||
| def github_request(path, token, params=None): | ||
| url = f"{GITHUB_API}{path}" | ||
| if params: | ||
| url += "?" + urllib.parse.urlencode(params) | ||
| request = urllib.request.Request(url, headers={ | ||
| "Authorization": f"Bearer {token}", | ||
| "Accept": "application/vnd.github+json", | ||
| "X-GitHub-Api-Version": "2022-11-28", | ||
| }) | ||
| with urllib.request.urlopen(request, timeout=REQUEST_TIMEOUT) as response: | ||
| return json.load(response) | ||
|
|
||
|
|
||
| def github_paginate(path, token, params=None, items_key=None): | ||
| page = 1 | ||
| while True: | ||
| page_params = dict(params or {}, per_page=100, page=page) | ||
| data = github_request(path, token, page_params) | ||
| items = data[items_key] if items_key else data | ||
| yield from items | ||
| if len(items) < 100: | ||
| return | ||
| page += 1 | ||
|
|
||
|
|
||
| def search_unreviewed_prs(org, token): | ||
| query = f"org:{org} is:pr (is:merged OR is:open) label:{LABEL}" | ||
| return list(github_paginate("/search/issues", token, {"q": query}, items_key="items")) | ||
|
|
||
|
|
||
| def label_anchor(repo, number, token): | ||
| """Timestamp of the latest UNREVIEWED 'labeled' event, or None.""" | ||
| latest = None | ||
| for event in github_paginate(f"/repos/{repo}/issues/{number}/timeline", token): | ||
| if event.get("event") == "labeled" and event.get("label", {}).get("name") == LABEL: | ||
| created = event.get("created_at") | ||
| if created and (latest is None or created > latest): | ||
| latest = created | ||
| if latest is None: | ||
| return None | ||
| return datetime.fromisoformat(latest.replace("Z", "+00:00")) | ||
|
|
||
|
|
||
| def nag_number(anchor_date, today): | ||
| """Count of weekdays d with anchor_date < d <= today. | ||
|
|
||
| Labeled Monday -> Tuesday run = 1; labeled Friday -> Monday run = 1 | ||
| (weekends don't count). Used as the "N weekdays unreviewed" display age. | ||
| """ | ||
| count = 0 | ||
| day = anchor_date | ||
| while day < today: | ||
| day = date.fromordinal(day.toordinal() + 1) | ||
| if day.weekday() < 5: | ||
| count += 1 | ||
| return count | ||
|
|
||
|
|
||
| def pr_targets(repo, number, author, token): | ||
| """(logins, is_author_fallback) — assignees, else requested reviewers, else the author. | ||
|
|
||
| Assignees win first: reassigning a PR to someone (e.g. a reviewer handing | ||
| it back to the author after requesting changes) signals who the ball is | ||
| with now, and requested_reviewers isn't cleared by submitting a review. | ||
| """ | ||
| pull = github_request(f"/repos/{repo}/pulls/{number}", token) | ||
| for team in pull.get("requested_teams", []): | ||
| print(f"SKIP team reviewer '{team['slug']}' on {repo}#{number} (teams unsupported)") | ||
| assignees = [user["login"] for user in pull.get("assignees", [])] | ||
| if assignees: | ||
| return assignees, False | ||
| reviewers = [user["login"] for user in pull.get("requested_reviewers", [])] | ||
| if reviewers: | ||
| return reviewers, False | ||
| return [author], True | ||
|
|
||
|
|
||
| def format_dm(entries): | ||
| lines = [ | ||
| "👋 You have pull requests waiting for review:", | ||
| "", | ||
| ] | ||
| for entry in entries: | ||
| nag = entry["nag"] | ||
| age = f"{nag} weekday{'s' if nag != 1 else ''} unreviewed" | ||
| lines.append(f"<{entry['url']}|{entry['title']}> ({entry['repo']}, {age})") | ||
| if entry["author_fallback"]: | ||
| lines.append(" ↳ your PR has *no reviewer assigned* — please request one") | ||
| lines += [ | ||
| "", | ||
| "_Review the PR, then remove the `UNREVIEWED` label to stop these reminders._", | ||
| ] | ||
| return "\n".join(lines) | ||
|
|
||
|
|
||
| def send_dm(slack_id, text, token): | ||
| payload = json.dumps({"channel": slack_id, "text": text}).encode() | ||
| request = urllib.request.Request(SLACK_API, data=payload, headers={ | ||
| "Authorization": f"Bearer {token}", | ||
| "Content-Type": "application/json; charset=utf-8", | ||
| }) | ||
| with urllib.request.urlopen(request, timeout=REQUEST_TIMEOUT) as response: | ||
| result = json.load(response) | ||
| if not result.get("ok"): | ||
| raise RuntimeError(f"Slack error for {slack_id}: {result.get('error')}") | ||
|
|
||
|
|
||
| def main(): | ||
| gh_token = os.environ["GH_TOKEN"] | ||
| org = os.environ.get("GH_ORG", "dreipol") | ||
| dry_run = os.environ.get("DRY_RUN", "true").lower() != "false" | ||
| slack_map = json.loads(os.environ.get("GH_SLACK_MAP") or "{}") | ||
| allowlist = json.loads(os.environ.get("ALLOWLIST") or "[]") | ||
|
|
||
| today = datetime.now(LOCAL_TZ).date() | ||
| prs = search_unreviewed_prs(org, gh_token) | ||
| print(f"Found {len(prs)} open/merged PRs with label {LABEL} (dry_run={dry_run})") | ||
|
|
||
| queue = {} # github login -> list of PR entries | ||
| for pr in prs: | ||
| repo = pr["repository_url"].removeprefix(f"{GITHUB_API}/repos/") | ||
| number = pr["number"] | ||
| anchor = label_anchor(repo, number, gh_token) | ||
| if anchor is None: | ||
| print(f"SKIP {repo}#{number}: no {LABEL} labeled event found") | ||
| continue | ||
| nag = nag_number(anchor.astimezone(LOCAL_TZ).date(), today) | ||
| if nag == 0: | ||
| print(f"SKIP {repo}#{number}: labeled today, first nag next Monday") | ||
| continue | ||
| targets, author_fallback = pr_targets(repo, number, pr["user"]["login"], gh_token) | ||
| for login in targets: | ||
| queue.setdefault(login, []).append({ | ||
| "repo": repo, "url": pr["html_url"], "title": pr["title"], | ||
| "nag": nag, "author_fallback": author_fallback, | ||
| }) | ||
|
|
||
| failures = 0 | ||
| for login, entries in sorted(queue.items()): | ||
| summary = ", ".join(f"{e['repo']}#{e['url'].rsplit('/', 1)[1]} (nag {e['nag']})" for e in entries) | ||
| if allowlist and login not in allowlist: | ||
| print(f"SKIP {login}: not on allowlist — {summary}") | ||
| continue | ||
| slack_id = slack_map.get(login) | ||
| if not slack_id: | ||
| print(f"SKIP {login}: no Slack mapping in GH_SLACK_MAP — {summary}") | ||
| continue | ||
| if dry_run: | ||
| print(f"DRY-RUN would DM {login} ({slack_id}): {summary}") | ||
| continue | ||
| try: | ||
| send_dm(slack_id, format_dm(entries), os.environ["SLACK_BOT_TOKEN"]) | ||
| print(f"SENT DM to {login} ({slack_id}): {summary}") | ||
| time.sleep(1) # chat.postMessage: ~1 msg/sec | ||
| except Exception as error: | ||
| print(f"ERROR DMing {login}: {error}") | ||
| failures += 1 | ||
|
|
||
| if failures: | ||
| sys.exit(f"{failures} Slack DM(s) failed") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ich denke es reicht, wenn man es aus
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aus welchem |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Copy this file into your repo as .github/workflows/pr-label.yml | ||
| # It labels every new PR with UNREVIEWED. Remove the label manually after | ||
| # you reviewed the PR — reminders stop once the label is gone (PR closed/merged) | ||
| # or removed. | ||
| name: PR label | ||
| on: | ||
| pull_request: | ||
| types: [opened, reopened] | ||
| permissions: | ||
| pull-requests: write | ||
| issues: write | ||
| jobs: | ||
| label: | ||
| uses: dreipol/github-actions/.github/workflows/reusable-pr-label.yml@master | ||
|
severindemchuk marked this conversation as resolved.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So ist es vermutlich einfacher, als erst beim Merge das Label hinzuzufügen. Dann aber bitte auch das Label automatisch entfernen, wenn der PR approved wird: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request_review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das war bewusst so entschieden, dass das entfernen vom Label impliziert das man fertig ist mit dem Review. Ausserdem kann man gemergte PRs nicht mehr approven nur kommentieren.