-
Notifications
You must be signed in to change notification settings - Fork 30
docs: update CONTRIBUTING.md with guidelines-bot details and review expectations #331
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
Open
PLeVasseur
wants to merge
1
commit into
rustfoundation:main
Choose a base branch
from
PLeVasseur:docs/update-contributing-md-with-bot-details
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+190
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,11 @@ | |||||
| - [7) Contributor Applies Regenerated Guideline to PR](#7-contributor-applies-regenerated-guideline-to-pr) | ||||||
| - [8) Your Guideline gets merged](#8-your-guideline-gets-merged) | ||||||
| - [You just contributed a coding guideline!](#you-just-contributed-a-coding-guideline) | ||||||
| - [Reviewer Bot Commands](#reviewer-bot-commands) | ||||||
| - [Overview](#overview) | ||||||
| - [Available Commands](#available-commands) | ||||||
| - [Review Deadlines](#review-deadlines) | ||||||
| - [Queue Status](#queue-status) | ||||||
| - [Writing a guideline locally (less typical, not recommended)](#writing-a-guideline-locally-less-typical-not-recommended) | ||||||
| - [Guideline template](#guideline-template) | ||||||
| - [Before You Begin Contributing](#before-you-begin-contributing) | ||||||
|
|
@@ -95,6 +100,8 @@ contents converted to reStructuredText. | |||||
|
|
||||||
| Within 14 days of your submission, a member of the Coding Guidelines Subcommittee should give you a first review. You'll work with them (and other members) to flesh out the concept and ensure the guideline is well prepared for a Pull Request. | ||||||
|
|
||||||
| > **Note:** A reviewer is automatically assigned from the pool of Producers using a round-robin system. See [Reviewer Bot Commands](#reviewer-bot-commands) for details. | ||||||
|
|
||||||
| ### 4) Create the PR | ||||||
|
|
||||||
| > [!NOTE] | ||||||
|
|
@@ -169,6 +176,189 @@ Once the coding guideline contents have passed review, a subcommittee member wil | |||||
|
|
||||||
| That's it! | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Reviewer Bot Commands | ||||||
|
|
||||||
| ### Overview | ||||||
|
|
||||||
| The reviewer bot (`@guidelines-bot`) automatically assigns reviewers to coding guideline issues and PRs using a round-robin system. Only members marked as "Producer" in the consortium's `subcommittee/coding-guidelines/members.md` are included in the rotation. | ||||||
|
|
||||||
| The queue state is stored in [Issue #314](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/314). | ||||||
|
Collaborator
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.
Suggested change
I'm 98% sure this goes like that. |
||||||
|
|
||||||
| ### Available Commands | ||||||
|
|
||||||
| All commands are invoked by mentioning `@guidelines-bot` in a comment: | ||||||
|
|
||||||
| #### Pass a Review (This Issue Only) | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /pass [optional reason] | ||||||
| ``` | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /pass Not familiar enough with FFI to review this one | ||||||
| ``` | ||||||
|
|
||||||
| #### Step Away from Queue | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /away YYYY-MM-DD [optional reason] | ||||||
| ``` | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /away 2025-02-15 On vacation | ||||||
| ``` | ||||||
|
|
||||||
| #### Claim a Review | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /claim | ||||||
| ``` | ||||||
|
|
||||||
| Use this to assign yourself as the reviewer for an issue/PR. This removes any existing reviewer assignment. Only Producers can claim reviews. | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /claim | ||||||
| ``` | ||||||
|
|
||||||
| #### Release Your Assignment | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /release [reason] | ||||||
| ``` | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /release Need to focus on other priorities | ||||||
| ``` | ||||||
|
|
||||||
| #### Assign a Specific Reviewer | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /r? @username | ||||||
| ``` | ||||||
|
|
||||||
| Use this to assign a specific person as the reviewer. This is useful when you know someone has specific expertise relevant to the guideline. | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /r? @expert-reviewer | ||||||
| ``` | ||||||
|
|
||||||
| #### Request Next Reviewer from Queue | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /r? producers | ||||||
| ``` | ||||||
|
|
||||||
| Use this to request the next reviewer from the round-robin queue for an already-open issue or PR. This is useful when: | ||||||
| - An issue/PR was opened without the `coding guideline` label and later labeled | ||||||
| - The original reviewer was removed and you need a new one | ||||||
| - You want to explicitly trigger the round-robin assignment | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /r? producers | ||||||
| ``` | ||||||
|
|
||||||
| #### Manage Labels | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /label +label-name # Add a label | ||||||
| @guidelines-bot /label -label-name # Remove a label | ||||||
| ``` | ||||||
|
|
||||||
| **Example:** | ||||||
| ``` | ||||||
| @guidelines-bot /label +needs-discussion | ||||||
| @guidelines-bot /label -ready-for-review | ||||||
| ``` | ||||||
|
|
||||||
| #### Sync Members | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /sync-members | ||||||
| ``` | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| #### Check Queue Status | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /queue | ||||||
| ``` | ||||||
|
|
||||||
| Shows the current queue position, who's next up for review, and who is currently away. | ||||||
|
|
||||||
| #### Show Available Commands | ||||||
|
|
||||||
| ``` | ||||||
| @guidelines-bot /commands | ||||||
| ``` | ||||||
|
|
||||||
| Shows all available bot commands with descriptions. | ||||||
|
|
||||||
| ### Review Deadlines | ||||||
|
|
||||||
| Reviewers have **14 days** to provide initial feedback on assigned issues or PRs. This timeline helps ensure contributors receive timely responses. | ||||||
|
|
||||||
| #### What Happens If the Deadline Passes | ||||||
|
|
||||||
| 1. **First 14 days**: The assigned reviewer should provide feedback or take action | ||||||
| 2. **After 14 days with no activity**: The bot posts a reminder and the reviewer enters a **14-day transition period** to Observer status | ||||||
| 3. **After 28 days total**: If still no activity, the reviewer may be transitioned from Producer to Observer status, and the review is reassigned | ||||||
|
|
||||||
| #### Acceptable Responses | ||||||
|
|
||||||
| Life happens! Any of these actions will reset the 14-day clock: | ||||||
|
|
||||||
| - **Post a review comment** - Any substantive feedback counts | ||||||
| - **Use `/pass [reason]`** - Pass the review to the next person if you can't review it | ||||||
| - **Use `/away YYYY-MM-DD [reason]`** - Step away temporarily (e.g., "On vacation until 2025-02-15") | ||||||
|
|
||||||
| #### Before You Pass: Consider the Learning Opportunity | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| Before reaching for `/pass`, we encourage you to spend about an hour engaging with the unfamiliar material: | ||||||
|
|
||||||
| - Skim the relevant FLS section and any linked documentation | ||||||
| - Read through the guideline with fresh eyes, noting what *does* make sense | ||||||
| - Search for a blog post or example that illuminates the concept | ||||||
| - Try compiling and tweaking the code examples yourself | ||||||
|
|
||||||
| 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?"—these contributions are valuable precisely *because* you're approaching the material without deep familiarity. | ||||||
|
|
||||||
| 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. | ||||||
|
|
||||||
| #### Examples of Valid Reasons to Pass | ||||||
|
|
||||||
| - "Not familiar enough with FFI to review this one" | ||||||
| - "On holiday, please assign to someone else" | ||||||
| - "Swamped with other work this week" | ||||||
|
|
||||||
| The goal is communication, not perfection. If you need to pass or step away, just let us know! | ||||||
|
|
||||||
| ### Queue Status | ||||||
|
|
||||||
| The queue state is stored in [Issue #314](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/314) and includes: | ||||||
|
|
||||||
| - **Current queue position** - Who will be assigned next | ||||||
| - **Active producers** - All reviewers in the rotation | ||||||
| - **Pass-until list** - Who is temporarily away and when they return | ||||||
| - **Recent assignments** - History of the last 20 assignments | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Writing a guideline locally (less typical, not recommended) | ||||||
|
|
||||||
|
|
||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We might want to clarify what we mean by a round-robin system.
I think I know what we mean here... something like this, perhaps?
Did I get it right?
If so, what happens if the queue is empty (because every producer is either on break or reviewing) and a new guideline proposal is opened?