Skip to content

feat(pr-workflow): add review-thread conduct and the core overlay - #176

Draft
MajorLift wants to merge 2 commits into
mainfrom
jongsun/docs/review-thread-conduct
Draft

MajorLift wants to merge 2 commits into
mainfrom
jongsun/docs/review-thread-conduct

Conversation

@MajorLift

Copy link
Copy Markdown
Contributor

Summary

  • Adds a Resolving Review Threads subsection under ## Reviewing Pull Requests in pr-guidelines' extension overlay: when an agent may resolve a thread, when to leave one open, and when not to re-post on an unresolved one.
  • Adds repos/core.md, which this skill did not have. tools/install skips a skill entirely when repos/ exists without a file for the target repo, so core was receiving nothing from a base skill whose own description covers reviewing someone else's PR.
  • Source: the ground rules established on metamask-extension#46123. The timing rule is hard, the rest are conventions, and each is worded as what it is.

Test plan

  • node --test test/*.test.mjs passes 71 of 71.
  • node .github/scripts/lint-skill-entry.mjs domains/pr-workflow/skills/pr-guidelines/skill.md reports 0 errors. The two warnings for missing recommended sections are pre-existing on a frontmatter-only skill.md.
  • An install targeting core now delivers pr-guidelines, where before it was skipped.

…rlay

`pr-guidelines` is a base skill whose description covers reviewing someone
else's PR, and it had no `repos/core.md`. `tools/install` skips a skill whose
`repos/` holds no file for the target repo, so `core` received none of it.
Resolving, minimizing and hiding all take a comment out of the default view,
which asserts the matter is settled. That judgement belongs to whoever raised
it, and an agent making it on a principal's behalf makes it in their name.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant