diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/core.md b/domains/pr-workflow/skills/pr-guidelines/repos/core.md new file mode 100644 index 00000000..22224cb9 --- /dev/null +++ b/domains/pr-workflow/skills/pr-guidelines/repos/core.md @@ -0,0 +1,16 @@ +--- +repo: core +parent: pr-guidelines +--- + +## Reviewing Pull Requests + +### Resolving Review Threads + +Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. + +- **Never resolve, minimize or hide a comment a human wrote, unless that human explicitly asks you to.** Collapsing a comment takes it out of the default view and asserts the matter is settled, which is a judgement belonging to whoever raised it. Doing it on a principal's behalf makes that assertion in their name. +- **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. +- **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. +- **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter. +- **Do not re-post on an unresolved thread that has not been updated.** Resolving every thread is not a merge requirement in this repo, unlike Mobile, so an unresolved thread is not by itself a signal that anything is outstanding. diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md index cc7eec91..c3b139f5 100644 --- a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md +++ b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md @@ -237,6 +237,16 @@ When NOT to use: - Non-blocking suggestions - Nitpicks +### Resolving Review Threads + +Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. + +- **Never resolve, minimize or hide a comment a human wrote, unless that human explicitly asks you to.** Collapsing a comment takes it out of the default view and asserts the matter is settled, which is a judgement belonging to whoever raised it. Doing it on a principal's behalf makes that assertion in their name. +- **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. +- **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. +- **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter. +- **Do not re-post on an unresolved thread that has not been updated.** Resolving every thread is not a merge requirement in this repo, unlike Mobile, so an unresolved thread is not by itself a signal that anything is outstanding. + ## Receiving Feedback ### Be Open to Other Perspectives