Skip to content
Open
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
50 changes: 50 additions & 0 deletions .github/reviewer-bot-tests/test_reviewer_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -888,6 +888,14 @@ def test_handle_issue_or_pr_opened_missing_label(stub_api, captured_comments, mo
assert captured_comments == []


def test_get_issue_guidance_includes_signoff_command_and_pr_ownership():
guidance = reviewer_bot.get_issue_guidance("alice", "dana")

assert "@guidelines-bot /label +sign-off: create pr" in guidance
assert "does not auto-create coding guideline PRs" in guidance
assert "stays open until that PR merges" in guidance


def test_handle_labeled_event_assigns_reviewer(stub_api, captured_comments, monkeypatch):
state = make_state()
os.environ["LABEL_NAME"] = "coding guideline"
Expand Down Expand Up @@ -940,6 +948,48 @@ def test_handle_labeled_event_sign_off_marks_completion(stub_api):
assert review_data["review_completion_source"] == "issue_label: sign-off: create pr"


def test_handle_labeled_event_sign_off_posts_transition_comment(captured_comments):
state = make_state()
state["active_reviews"]["42"] = {
"current_reviewer": "alice",
"assigned_at": "2000-01-01T00:00:00+00:00",
"last_reviewer_activity": "2000-01-01T00:00:00+00:00",
}
os.environ["LABEL_NAME"] = "sign-off: create pr"
os.environ["ISSUE_NUMBER"] = "42"
os.environ["ISSUE_AUTHOR"] = "dana"

handled = reviewer_bot.handle_labeled_event(state)

assert handled is True
assert len(captured_comments) == 1
body = captured_comments[0]["body"]
assert "issue is now PR-ready" in body
assert "@dana" in body
assert "`closes #42`" in body
assert "Keep this issue open until that PR merges." in body


def test_handle_labeled_event_sign_off_already_complete_no_transition_comment(captured_comments):
state = make_state()
state["active_reviews"]["42"] = {
"current_reviewer": "alice",
"assigned_at": "2000-01-01T00:00:00+00:00",
"last_reviewer_activity": "2000-01-01T00:00:00+00:00",
"review_completed_at": "2000-01-02T00:00:00+00:00",
"review_completed_by": "alice",
"review_completion_source": "issue_label: sign-off: create pr",
}
os.environ["LABEL_NAME"] = "sign-off: create pr"
os.environ["ISSUE_NUMBER"] = "42"
os.environ["ISSUE_AUTHOR"] = "dana"

handled = reviewer_bot.handle_labeled_event(state)

assert handled is False
assert captured_comments == []


def test_handle_labeled_event_sign_off_ignored_for_pr(stub_api):
state = make_state()
state["active_reviews"]["42"] = {
Expand Down
26 changes: 22 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ If you are new here, this is the shortest path to a first PR. The detailed workf
3. Wait for the reStructuredText comment from the bot. See [2) Guideline Generated as a Comment](#2-guideline-generated-as-a-comment).
4. Collaborate with a subcommittee member to refine the draft and get `sign-off: create pr`. See [3) Create a Draft with a Member](#3-create-a-draft-with-a-member).
5. Create the PR using the generated RST and include `closes #xyz` in the PR body. See [4) Create the PR](#4-create-the-pr).
6. Iterate on PR feedback until it is approved and merged. See [5) Iterate on Feedback](#5-iterate-on-feedback).
6. Iterate on PR feedback until it is approved, queued, and merged. See [5) Iterate on Feedback](#5-iterate-on-feedback).

## Contribution Workflow

Expand All @@ -44,12 +44,12 @@ flowchart TD

Issue --> S4{{"4: Contributor creates a PR using the reStructuredText generated for them on issue"}} --> PR["Coding Guideline<br>Pull Request"]

S5{{"5: <br> 5.1 PR review started by subcommittee member in <= 14 days <br><br> 5.2 Contributor discusses on PR with members and updates"}} --> PR
S5{{"5: <br> 5.1 PR review started by subcommittee member in <= 14 days <br><br> 5.2 Contributor discusses on PR with members and updates <br><br> 5.3 Approved PR is queued by someone with write permissions"}} --> PR
PR --> S5

PR --> S6{{"(Optional) <br> 6: Contributor applies feedback to issue"}} --> Issue
Issue --> S7{{"(Optional)<br> 7: Contributor applies updated reStructuredText to Pull request"}} --> PR
PR --> S8{{"8: Subcommittee member <br> approves & queues;<br>merges to main"}} --> Main[[main]]
PR --> S8{{"8: Producer reviewer approves;<br>someone with write permissions queues;<br>merges to main"}} --> Main[[main]]
Main --> End(["9: End"])
```

Expand Down Expand Up @@ -105,6 +105,10 @@ Within 14 days of your submission, a member of the Coding Guidelines Subcommitte

When a subcommittee member adds the `sign-off: create pr` label, the issue review is considered complete and reviewer reminders stop.

> [!IMPORTANT]
> `sign-off: create pr` means the issue is ready for a Pull Request.
> It does **not** mean the Pull Request is approved or queued for merge.

### 4) Create the PR

> [!NOTE]
Expand All @@ -120,6 +124,8 @@ When a subcommittee member adds the `sign-off: create pr` label, the issue revie

As soon as these prerequisites are fulfilled, the draft shall be marked as PR-ready by a subcommittee member, by labeling the issue with `sign-off: create pr`. This denotes that you should create a Pull Request with your Guideline. Further discussion about the amount and correctness of its content shall then be done on the Pull Request itself.

Contributors open their own Pull Requests after sign-off. The reviewer marks the issue PR-ready but does not create the coding guideline PR automatically.

The contents of the PR should be based on the bot comment containing the generated RST form of your guideline, as seen in [Step 2](#2-guideline-generated-as-a-comment). The comment has the exact file content you'll need.

In order to ensure your guideline appears when rendering the document, reference the generated comment from [Step 2](#2-guideline-generated-as-a-comment). All the steps necessary should appear below the headings `📁 Target Location` and `🗂️ Update Chapter Index`.
Expand Down Expand Up @@ -152,6 +158,18 @@ If you agree with the suggested changes, you've got two options:
and
[7) Contributor Applies Regenerated Guideline to PR](#7-contributor-applies-regenerated-guideline-to-pr)

#### 5.3) PR Approval and Merge Queue

Once the pull request content is ready, the expected merge flow is:

1. The assigned Producer reviewer approves the pull request.
2. Someone with write permissions adds the approved pull request to the merge queue.
3. GitHub runs the required checks for queued merges (including `build`).
4. GitHub merges the pull request automatically after required checks pass.
5. If the PR body includes `closes #xyz`, GitHub closes that issue when the PR merges.

If your PR is approved but not yet queued, comment on the PR and ask a maintainer or Producer with write permissions to queue it.

### 6) Contributor Applies Feedback on Issue

(Optional, if not comfortable with reStructured Text from
Expand All @@ -173,7 +191,7 @@ reflected into the Pull Request.

### 8) Your Guideline gets merged

Once the coding guideline contents have passed review, a subcommittee member will approve the pull request, and put it on the merge queue to be merged.
Once the coding guideline contents have passed review, the pull request is approved by a Producer reviewer and queued by someone with write permissions. GitHub then merges it to `main` automatically after required checks pass.

### You just contributed a coding guideline!

Expand Down
21 changes: 21 additions & 0 deletions REVIEWING.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,27 @@ Shows the current queue position, who's next up for review, and who is currently

Shows all available bot commands with descriptions.

## PR Merge Handoff

Once a coding guideline PR is content-complete, use the following handoff flow:

| Stage | Primary actor | Action | Completion signal |
| --- | --- | --- | --- |
| Issue draft review | Producer | Run `@guidelines-bot /label +sign-off: create pr` on the issue | Issue is PR-ready |
| PR content review | Assigned Producer reviewer | Approve the PR | Review is complete |
| Merge execution | Maintainer or Producer with write permissions | Add approved PR to merge queue | PR is merged to `main` |

Important distinctions:

- `sign-off: create pr` means the issue is ready for a PR. It does not mean merge approval.
- Contributors open their own PRs after sign-off; reviewers do not auto-create coding guideline PRs.
- PR approval and queueing are separate actions.
- The issue remains open at sign-off and closes when the linked PR merges (for example, via `closes #xyz` in the PR body).
- If the assigned reviewer does not have write permissions, ask in the PR thread for a maintainer or Producer with write permissions to queue the PR.
- Queued PRs merge automatically after required checks pass (including `build`).

If the assigned reviewer has already approved, but reviewer-bot state still looks incomplete, run `@guidelines-bot /rectify`.

## Review Deadlines

Reviewers have **14 days** to provide initial feedback on assigned issues or PRs. This timeline helps ensure contributors receive timely responses.
Expand Down
39 changes: 37 additions & 2 deletions scripts/reviewer_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -1831,7 +1831,9 @@ def get_issue_guidance(reviewer: str, issue_author: str) -> str:
- Content written may be *incomplete*, but must not be *incorrect*
- The `🧪 Code Example Test Results` section shows all example code compiles

4. When ready, **add the `sign-off: create pr` label** to signal the contributor should create a PR
4. When ready, run **`{BOT_MENTION} /label +sign-off: create pr`** to mark the issue PR-ready
5. This signals @{issue_author} to open a Pull Request; the bot does not auto-create coding guideline PRs
6. The issue stays open until that PR merges

## Bot Commands

Expand All @@ -1854,6 +1856,27 @@ def get_issue_guidance(reviewer: str, issue_author: str) -> str:
"""


def get_issue_signoff_transition_comment(issue_number: int, issue_author: str) -> str:
"""Generate follow-up guidance when an issue is marked PR-ready."""
if issue_author:
contributor_line = (
f"@{issue_author}, please open a Pull Request using the generated RST "
"comment from this issue."
)
else:
contributor_line = (
"Please open a Pull Request using the generated RST comment from this issue."
)

return (
"✅ This issue is now PR-ready (`sign-off: create pr`).\n\n"
f"{contributor_line}\n\n"
"When you open the PR:\n"
f"- Include `closes #{issue_number}` in the PR body so this issue closes when the PR merges.\n"
"- Keep this issue open until that PR merges."
)


def get_fls_audit_guidance(reviewer: str, issue_author: str) -> str:
"""Generate guidance text for an FLS audit issue reviewer."""
return f"""👋 Hey @{reviewer}! You've been assigned to review this FLS audit issue.
Expand Down Expand Up @@ -3713,12 +3736,24 @@ def handle_labeled_event(state: dict) -> bool:
reviewer = None
if review_data:
reviewer = review_data.get("current_reviewer")
return mark_review_complete(
review_marked_complete = mark_review_complete(
state,
issue_number,
reviewer,
"issue_label: sign-off: create pr",
)
if review_marked_complete:
issue_author = os.environ.get("ISSUE_AUTHOR", "").strip()
transition_comment = get_issue_signoff_transition_comment(
issue_number,
issue_author,
)
if not post_comment(issue_number, transition_comment):
print(
f"WARNING: Failed to post sign-off transition comment on #{issue_number}",
file=sys.stderr,
)
return review_marked_complete

if label_name not in REVIEW_LABELS:
print(f"Label '{label_name}' is not a review label, skipping")
Expand Down