|
| 1 | +# Reviewing coding guidelines |
| 2 | + |
| 3 | +This document is for members reviewing coding guideline issues and PRs (Producers). |
| 4 | +If you are contributing a guideline, start with [CONTRIBUTING.md](CONTRIBUTING.md). |
| 5 | + |
| 6 | +## Reviewer Bot Commands |
| 7 | + |
| 8 | +> [!NOTE] |
| 9 | +> These commands only apply in the context of coding guideline issues. |
| 10 | +
|
| 11 | +Before we continue, here's a preamble on how the reviewer bot helps reviewers do their job. |
| 12 | + |
| 13 | +1. The reviewer bot (`guidelines-bot`) automatically assigns reviewers to coding guideline issues and PRs using a round-robin system. |
| 14 | + |
| 15 | +2. Only members marked as "Producer" in the consortium's [`members.md`](https://github.com/rustfoundation/safety-critical-rust-consortium/blob/main/subcommittee/coding-guidelines/members.md) are included in the rotation. |
| 16 | + |
| 17 | +3. The queue's state is stored in [Issue #314](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/314). |
| 18 | + |
| 19 | +4. All commands are invoked by mentioning `@guidelines-bot` in a comment. |
| 20 | + |
| 21 | +Round-robin here means the bot maintains a queue of Producers and a `current_index` cursor. Each assignment takes the next eligible reviewer in queue order and advances the cursor; the queue order does not change, except when `/pass` repositions the reviewer to be next up for future assignments. If no eligible reviewer is available (queue empty or all candidates skipped), the bot leaves the issue or PR unassigned and posts: "No reviewers available in the queue. Please use `@guidelines-bot /sync-members` to update the queue." |
| 22 | + |
| 23 | +Down below are the available commands. |
| 24 | + |
| 25 | +### Pass this Review to the next Producer |
| 26 | + |
| 27 | +``` |
| 28 | +@guidelines-bot /pass [optional reason] |
| 29 | +``` |
| 30 | + |
| 31 | +Use this when you cannot review a specific issue/PR but want to remain in the rotation for future assignments. The next reviewer in the queue will be assigned instead. |
| 32 | + |
| 33 | +**Example:** |
| 34 | +``` |
| 35 | +@guidelines-bot /pass Not familiar enough with FFI to review this one |
| 36 | +``` |
| 37 | + |
| 38 | +### Step Away from Queue |
| 39 | + |
| 40 | +``` |
| 41 | +@guidelines-bot /away YYYY-MM-DD [optional reason] |
| 42 | +``` |
| 43 | + |
| 44 | +Use this to temporarily remove yourself from the reviewer queue until the specified date. You'll be automatically added back when the date arrives. If you're currently assigned to an issue/PR, the next reviewer will be assigned. |
| 45 | + |
| 46 | +**Example:** |
| 47 | +``` |
| 48 | +@guidelines-bot /away 2025-02-15 On vacation |
| 49 | +``` |
| 50 | + |
| 51 | +### Claim a Review |
| 52 | + |
| 53 | +``` |
| 54 | +@guidelines-bot /claim |
| 55 | +``` |
| 56 | + |
| 57 | +Use this to assign yourself as the reviewer for an issue/PR. This removes any existing reviewer assignment. Only Producers can claim reviews. |
| 58 | + |
| 59 | +**Example:** |
| 60 | +``` |
| 61 | +@guidelines-bot /claim |
| 62 | +``` |
| 63 | + |
| 64 | +### Release Your Assignment |
| 65 | + |
| 66 | +``` |
| 67 | +@guidelines-bot /release [reason] |
| 68 | +``` |
| 69 | + |
| 70 | +Use this to release your assignment from an issue/PR without automatically assigning someone else. The issue/PR will be left unassigned. Use `/pass` if you want to automatically assign the next reviewer. |
| 71 | + |
| 72 | +**Example:** |
| 73 | +``` |
| 74 | +@guidelines-bot /release Need to focus on other priorities |
| 75 | +``` |
| 76 | + |
| 77 | +### Rectify Review State |
| 78 | + |
| 79 | +``` |
| 80 | +@guidelines-bot /rectify |
| 81 | +``` |
| 82 | + |
| 83 | +Use this when GitHub review activity and reviewer-bot state appear out of sync. |
| 84 | +This is most common on cross-repository PRs where a review event may not have had |
| 85 | +permission to persist reviewer-bot state. |
| 86 | + |
| 87 | +Who can run it: |
| 88 | +- The currently assigned reviewer |
| 89 | +- A maintainer with triage+ permission |
| 90 | + |
| 91 | +What it does (for the current PR only): |
| 92 | +- If the latest review by the assigned reviewer is `APPROVED`, it marks the review complete. |
| 93 | +- If the latest review by the assigned reviewer is `COMMENTED` or `CHANGES_REQUESTED`, it refreshes reviewer activity. |
| 94 | +- Otherwise, it reports that no reconciliation transition applies. |
| 95 | + |
| 96 | +**Example:** |
| 97 | +``` |
| 98 | +@guidelines-bot /rectify |
| 99 | +``` |
| 100 | + |
| 101 | +### Assign a Specific Reviewer |
| 102 | + |
| 103 | +``` |
| 104 | +@guidelines-bot /r? @username |
| 105 | +``` |
| 106 | + |
| 107 | +Use this to assign a specific person as the reviewer. This is useful when you know someone has specific expertise relevant to the guideline. |
| 108 | + |
| 109 | +**Example:** |
| 110 | +``` |
| 111 | +@guidelines-bot /r? @expert-reviewer |
| 112 | +``` |
| 113 | + |
| 114 | +### Request Next Reviewer from Queue |
| 115 | + |
| 116 | +``` |
| 117 | +@guidelines-bot /r? producers |
| 118 | +``` |
| 119 | + |
| 120 | +Use this to request the next reviewer from the round-robin queue for an already-open issue or PR. This is useful when: |
| 121 | +- An issue/PR was opened without the `coding guideline` label and later labeled |
| 122 | +- The original reviewer was removed and you need a new one |
| 123 | +- You want to explicitly trigger the round-robin assignment |
| 124 | + |
| 125 | +**Example:** |
| 126 | +``` |
| 127 | +@guidelines-bot /r? producers |
| 128 | +``` |
| 129 | + |
| 130 | +### Manage Labels |
| 131 | + |
| 132 | +``` |
| 133 | +@guidelines-bot /label +label-name # Add a label |
| 134 | +@guidelines-bot /label -label-name # Remove a label |
| 135 | +``` |
| 136 | + |
| 137 | +**Example:** |
| 138 | +``` |
| 139 | +@guidelines-bot /label +needs-discussion |
| 140 | +@guidelines-bot /label -ready-for-review |
| 141 | +``` |
| 142 | + |
| 143 | +### Sync Members |
| 144 | + |
| 145 | +``` |
| 146 | +@guidelines-bot /sync-members |
| 147 | +``` |
| 148 | + |
| 149 | +Manually trigger a sync of the reviewer queue with `members.md`. This happens automatically on each workflow run, but you can force it if needed. |
| 150 | + |
| 151 | +### Check Queue Status |
| 152 | + |
| 153 | +``` |
| 154 | +@guidelines-bot /queue |
| 155 | +``` |
| 156 | + |
| 157 | +Shows the current queue position, who's next up for review, and who is currently away. |
| 158 | + |
| 159 | +### Show Available Commands |
| 160 | + |
| 161 | +``` |
| 162 | +@guidelines-bot /commands |
| 163 | +``` |
| 164 | + |
| 165 | +Shows all available bot commands with descriptions. |
| 166 | + |
| 167 | +## Review Deadlines |
| 168 | + |
| 169 | +Reviewers have **14 days** to provide initial feedback on assigned issues or PRs. This timeline helps ensure contributors receive timely responses. |
| 170 | + |
| 171 | +Review comments or changes requested by the assigned reviewer reset the 14-day timer. When the assigned reviewer approves the PR, the review is marked complete and reminders stop. |
| 172 | + |
| 173 | +### What Happens If the Deadline Passes |
| 174 | + |
| 175 | +1. **First 14 days**: The assigned reviewer should provide feedback or take action |
| 176 | +2. **After 14 days with no activity**: The bot posts a reminder and the reviewer enters a **14-day transition period** to Observer status |
| 177 | +3. **After 28 days total**: If still no activity, the reviewer may be transitioned from Producer to Observer status, and the review is reassigned |
| 178 | + |
| 179 | +### Acceptable Responses |
| 180 | + |
| 181 | +Life happens! Any of these actions will reset the 14-day clock: |
| 182 | + |
| 183 | +- **Post a review comment** - Any substantive feedback counts |
| 184 | +- **Use `/pass [reason]`** - Pass the review to the next person if you can't review it |
| 185 | +- **Use `/away YYYY-MM-DD [reason]`** - Step away temporarily (e.g., "On vacation until 2025-02-15") |
| 186 | +- **Use `/rectify`** - Reconcile PR review state when review activity happened but bot state is stale |
| 187 | + |
| 188 | +#### Before You Pass: Consider the Learning Opportunity |
| 189 | + |
| 190 | +Being assigned a review outside your comfort zone can feel daunting, but it's also one of the most effective ways to deepen your Rust knowledge. When you have a concrete goal, understanding this guideline about this feature, learning becomes focused and sticky in a way that abstract study rarely achieves. |
| 191 | + |
| 192 | +Before reaching for `/pass`, we encourage you to spend about an hour engaging with the unfamiliar material: |
| 193 | + |
| 194 | +- Skim the relevant FLS section and any linked documentation |
| 195 | +- Read through the guideline with fresh eyes, noting what *does* make sense |
| 196 | +- Search for a blog post or example that illuminates the concept |
| 197 | +- Try compiling and tweaking the code examples yourself |
| 198 | + |
| 199 | +You may find that an hour of targeted exploration is enough to provide meaningful feedback, even if you're not an expert. Catching unclear explanations, spotting typos, or asking "what does this term mean?" are contributions that matter because you're approaching the material without deep familiarity. |
| 200 | + |
| 201 | +That said, `/pass` exists for good reason. If after an honest effort the material remains opaque, or if the guideline requires genuine expertise you don't have (and can't reasonably acquire in an hour), passing to someone better suited is the right call. The goal is thoughtful engagement, not struggling through a review you can't meaningfully contribute to. |
| 202 | + |
| 203 | +#### Examples of Valid Reasons to Pass |
| 204 | + |
| 205 | +- "Not familiar enough with FFI to review this one" |
| 206 | +- "On holiday, please assign to someone else" |
| 207 | +- "Swamped with other work this week" |
| 208 | + |
| 209 | +The goal is communication, not perfection. If you need to pass or step away, just let us know! |
| 210 | + |
| 211 | +## Queue Status |
| 212 | + |
| 213 | +The queue's state is stored in [Issue #314](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/314) and includes: |
| 214 | + |
| 215 | +- **Current queue position** - Who will be assigned next |
| 216 | +- **Active producers** - All reviewers in the rotation |
| 217 | +- **Pass-until list** - Who is temporarily away and when they return |
| 218 | +- **Recent assignments** - History of the last 20 assignments |
0 commit comments