You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
If the PR body links an issue (from Step 1's PR details), search for other open PRs that reference the same issue:
71
+
72
+
```bash
73
+
gh pr list --repo medusajs/medusa --state open --search "issue-number" --json number,title,author
74
+
```
75
+
76
+
If another open PR is found that links the same issue:
77
+
1. Include a **Heads up** note at the top of your review comment (before any other feedback):
78
+
```
79
+
**Heads up for the team:** PR #<other_number> also references issue #<issue_number>. Please coordinate to avoid duplicated work.
80
+
```
81
+
2. This note is **informational only** — it does not change the label outcome. Continue the rest of the review as normal.
82
+
83
+
If the PR doesn't link an issue, skip this step.
84
+
85
+
> **CRITICAL:** Do not block or close the PR solely because a duplicate was found. Only flag it so the team or author can decide how to proceed.
86
+
87
+
### Step 3 — Review Prior Comments
69
88
70
89
Read the existing comments fetched in Step 1. Identify any previous bot review comments (comments from this automation) that raised issues or made requests.
71
90
@@ -78,11 +97,11 @@ For each previously raised issue or remark, determine whether it has been addres
78
97
79
98
> **CRITICAL:** Do not repeat the full explanation for issues already raised in a previous comment. Keep follow-up reviews concise — assume the contributor has read the prior feedback.
80
99
81
-
### Step 3 — Check Team Membership
100
+
### Step 4 — Check Team Membership
82
101
83
-
Read `.github/teams.yml`. If the PR author's login appears in the list, they are a **team member** — **skip steps 3 and 4** entirely and proceed directly to step 5.
102
+
Read `.github/teams.yml`. If the PR author's login appears in the list, they are a **team member** — **skip steps 4 and 5** entirely and proceed directly to step 6.
84
103
85
-
### Step 3 — Template Compliance (non-team members only)
104
+
### Step 5 — Template Compliance (non-team members only)
86
105
87
106
The PR body must follow `.github/pull_request_template.md`. It requires these sections to be filled in (not left as placeholder text):
88
107
@@ -104,7 +123,7 @@ Once you've updated the description, another review will be triggered automatica
104
123
Thanks!
105
124
```
106
125
107
-
### Step 4 — Non-Member Checks (skip if team member)
126
+
### Step 6 — Non-Member Checks (skip if team member)
Check whether any linked issue carries a `help-wanted` label. If not, apply `requires-more` and comment explaining that large contributions should be scoped and pre-approved via an issue first (reference `CONTRIBUTING.md`).
116
135
117
-
### Step 5 — Fetch Linked Issues
136
+
### Step 7 — Fetch Linked Issues
118
137
119
138
```bash
120
139
bash scripts/get_linked_issues.sh <pr_number>
121
140
```
122
141
123
142
Look for closing keywords (`closes`, `fixes`, `resolves` + `#<number>`) in the PR body. Note whether a verified, open issue is linked.
124
143
125
-
### Step 6 — Determine Contribution Type
144
+
### Step 8 — Determine Contribution Type
126
145
127
146
Inspect the changed file paths and load the relevant reference section:
128
147
@@ -134,15 +153,15 @@ Inspect the changed file paths and load the relevant reference section:
134
153
135
154
For mixed PRs, apply all relevant types.
136
155
137
-
### Step 7 — Check Conventions
156
+
### Step 9 — Check Conventions
138
157
139
158
Load `reference/conventions.md` and verify the changed files follow Medusa's conventions. Focus on the areas most relevant to the contribution type (e.g., API conventions for code changes, MDX structure for docs changes).
140
159
141
160
> **CRITICAL — Read full file context:** For every file you intend to flag an issue in, read the **entire file** (not just the diff lines) before raising a concern. A pattern that looks wrong in isolation may be handled correctly later in the file, overridden by a wrapper, or follow an established project convention. Only flag an issue after confirming it is not resolved elsewhere in the file.
142
161
143
162
> **CRITICAL — Only flag new code:** The diff contains both removed lines (prefixed `-`) and added lines (prefixed `+`). **Only raise issues about added/new lines.** Never request changes to lines that were already modified as part of this PR — the new version of those lines is what matters, not the old one.
144
163
145
-
### Step 8 — Security Analysis (ALL PRs)
164
+
### Step 10 — Security Analysis (ALL PRs)
146
165
147
166
> **CRITICAL:** This step applies to **all PRs**, including team members. Read the actual diff — do not rely only on file path inspection. Before flagging any issue, read the full file to confirm the concern is not already handled elsewhere. Only flag issues present in the new (added) lines of the diff, not in lines that were removed or already changed by this PR.
148
167
@@ -187,7 +206,7 @@ For each confirmed or suspected security issue:
187
206
188
207
Security issues are always **blocking** — apply `requires-more` even if everything else looks good. Load `reference/comment-guidelines.md` for the Security Issues comment format.
189
208
190
-
### Step 9 — Performance Analysis (ALL PRs)
209
+
### Step 11 — Performance Analysis (ALL PRs)
191
210
192
211
> **CRITICAL:** This step applies to **all PRs**. Only flag issues that would plausibly cause measurable degradation in production — not theoretical micro-optimizations. Before flagging, read the full file to confirm the issue is not already handled elsewhere. Only flag issues in the new (added) lines of the diff.
193
212
@@ -218,7 +237,7 @@ Performance issues severity:
218
237
-**Blocking (requires-more):** N+1 queries, unbounded queries on large tables, missing pagination on list endpoints
219
238
-**Non-blocking (note only):** Suggestions that are improvements but don't introduce clear production risk
220
239
221
-
### Step 10 — Bug Detection (ALL PRs)
240
+
### Step 12 — Bug Detection (ALL PRs)
222
241
223
242
> **CRITICAL:** This step applies to **all PRs** including team members. Any potential bug — confirmed or suspected — is a **required change** and must result in `requires-more`. Do not leave bugs as notes.
224
243
@@ -247,7 +266,7 @@ For each potential bug found:
247
266
248
267
> Do NOT flag style issues, code smell, or naming preferences here. Only flag things that would plausibly cause incorrect behaviour at runtime. If you're uncertain, phrase it as a question but still add it to **Required changes** — it is the author's responsibility to confirm or disprove it.
249
268
250
-
### Step 11 — Contextual Assessment
269
+
### Step 13 — Contextual Assessment
251
270
252
271
Before writing the review, assess whether the changes make sense in the broader context of the PR. Load `reference/comment-guidelines.md` (Contextual Assessment section) for the full checklist. Key questions:
253
272
@@ -258,7 +277,7 @@ Before writing the review, assess whether the changes make sense in the broader
258
277
259
278
Note any concerns to include in the review comment.
260
279
261
-
### Step 12 — Compose and Post Review
280
+
### Step 14 — Compose and Post Review
262
281
263
282
Load `reference/comment-guidelines.md` for comment templates and tone guidance.
-[ ] Checking template compliance for team members — skip for team members
292
311
-[ ] Being vague about required changes — always list exactly what needs to change and why
312
+
-[ ] Approving a PR that changes behavior documented as intentional — always check the docs when a PR modifies existing behavior; if the docs describe it as by design, flag it as `requires-more`
293
313
-[ ] Closing a PR without a clear explanation
294
314
-[ ] Forgetting the docs-ui test requirement for `www/packages/docs-ui/` changes
295
315
-[ ] Skipping the integration test check for API route changes in `packages/medusa/src/api/`
Copy file name to clipboardExpand all lines: .claude/skills/reviewing-prs/reference/comment-guidelines.md
+1Lines changed: 1 addition & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -84,6 +84,7 @@ Before composing the review, assess whether the changes make sense in the broade
84
84
85
85
**Ask yourself:**
86
86
87
+
- **Is the behavior being changed intentional and documented?** If the PR modifies existing behavior, check whether that behavior is described as by design in the official Medusa documentation (`www/apps/book/app/learn/`). If the docs explicitly describe the current behavior as intentional, the PR is changing intended behavior and must be flagged as `requires-more`. Explain that the behavior is by design and reference the documentation section. This is a **blocking** concern — do not apply `initial-approval`.
87
88
- **Does it make sense?** Does the implementation actually solve the problem described in the PR or linked issue? Is the approach reasonable, or is there a simpler/safer way?
88
89
- **Are there unintended side effects?** Could the change break or alter behaviour in other areas of the codebase? For example: shared utilities, middleware, event handlers, or widely-used types.
89
90
- **Is the scope right?** Does the PR do more or less than what the linked issue asks for? Extra unrelated changes are a flag.
0 commit comments