docs: reply to each review comment in its own thread, not in a rollup (#2254)
pr-flow §8's "mirror each round at PR level" was being read as permission to skip the inline reply, so the rollup got posted and the per-thread replies never happened. A bullet elsewhere on the page cannot be connected back to the thread it answers: the thread stays open showing a finding and silence, and by round three matching bullets to comments is reconstruction rather than reading. - AGENTS.md, "Responding to Code Reviews": says where the response goes, which was the ambiguity being resolved the wrong way — inline in the comment's own thread, with the rollup in addition, never instead. - pr-flow §8 reorders the two so the primary is unmistakable, and keeps the reason the mirror exists (threads go outdated once the fix is pushed) as a rationale for it rather than an alternative to the inline reply. - Both name the exception: a finding in the "Suppressed comments" block has no comment id and so no thread to reply into, making the mirror its only home. - §8 gains the two mechanics, so the easy path is not the only documented one: the reply endpoint keyed by comment id, and fetching comments per review by review id, since the unpaginated /reviews listing hides later rounds behind your own replies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017nLLLTGc37hQiC8s3mNyhq Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
ed622396b9
commit
f504b89b56
2 changed files with 29 additions and 5 deletions
|
|
@ -279,11 +279,34 @@ arrive late (see step 8).
|
|||
|
||||
- It is **not** necessary to implement every suggestion. Implementing one a
|
||||
different way, or declining it with a reason, is fine.
|
||||
- After making the changes, **respond to each comment** with what was done, or
|
||||
why it was ignored.
|
||||
- ⚠️ **Inline replies go hidden once the fix is pushed** (the threads become
|
||||
outdated), so **mirror each round at PR level** as a summary comment, and always
|
||||
read the "Suppressed comments" block.
|
||||
- After making the changes, **reply to each review comment in its own thread**
|
||||
with what was done, or why it was declined. That inline reply is the primary
|
||||
response and it is not optional — each review comment is a discussion thread
|
||||
with its own resolve state, and only a reply _in_ the thread can resolve it or
|
||||
show a reviewer reading it that the finding was answered.
|
||||
|
||||
```sh
|
||||
# Fetch the round's comments by REVIEW id — the unpaginated /reviews listing
|
||||
# hides later rounds behind your own replies.
|
||||
gh api repos/modelcontextprotocol/inspector/pulls/<N>/reviews/<REVIEW_ID>/comments \
|
||||
--jq '.[]|"\(.id) \(.path):\(.line)\n\(.body)"'
|
||||
|
||||
# Reply into one thread, keyed by the comment id from above.
|
||||
gh api repos/modelcontextprotocol/inspector/pulls/<N>/comments/<COMMENT_ID>/replies \
|
||||
-f body='Fixed in <sha> — …'
|
||||
```
|
||||
|
||||
- ⚠️ **Then mirror the round at PR level, in addition — never instead.** Inline
|
||||
replies go hidden once the fix is pushed, because the threads become outdated,
|
||||
so a summary comment is what keeps the round readable afterwards. It does
|
||||
**not** discharge the per-comment replies: a rollup bullet cannot be connected
|
||||
back to the thread it answers, so the thread stays open with a finding and
|
||||
silence in it, and by round three matching bullets to comments is
|
||||
reconstruction rather than reading.
|
||||
- ⚠️ Always read the **"Suppressed comments"** block in the review body. Those
|
||||
findings have no comment id, so they have no thread to reply into — the
|
||||
PR-level mirror is the only place they can be answered, and it is the one case
|
||||
where answering there is the whole response.
|
||||
- ⚠️ **Copilot's inline comments lag its review body.** The body's "generated N
|
||||
comments" count lands first; fetch by recency and reconcile. Repeated
|
||||
re-review silence means the session ended.
|
||||
|
|
|
|||
|
|
@ -372,6 +372,7 @@ When asked to respond to a code review of a PR:
|
|||
- it is not necessary to implement all suggestions
|
||||
- you are free to implement suggestions in a different way, or to ignore one if there is a good reason
|
||||
- after making the changes, respond to each review comment with what was done (or why it was ignored)
|
||||
- **that response goes in the review comment's own thread — a rollup comment does not discharge it.** Each review comment is a discussion thread with its own resolve state, so a bullet posted elsewhere on the page cannot be connected back to the thread it answers: the thread stays open showing a finding and no reply, and the PR reads as though the review were ignored. Reply inline first, per comment; then post the PR-level summary **in addition**, because inline replies go hidden once the fix is pushed. A finding in the review's "Suppressed comments" block has no thread to reply into, so the summary is the only place it can be answered — that is the one exception. The `gh` calls are in the `pr-flow` skill, step 8.
|
||||
|
||||
## Always test new or modified code
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue