Skip to content

Adds PR review skill and checklist for pull request evaluations#7227

Merged
milanholemans merged 11 commits into
pnp:mainfrom
waldekmastykarz:pr-review-skill
Jul 15, 2026
Merged

Adds PR review skill and checklist for pull request evaluations#7227
milanholemans merged 11 commits into
pnp:mainfrom
waldekmastykarz:pr-review-skill

Conversation

@waldekmastykarz

Copy link
Copy Markdown
Member

No description provided.

@Jwaegebaert

Copy link
Copy Markdown
Contributor

Isn't this simular to #7193?

@waldekmastykarz

Copy link
Copy Markdown
Member Author

Yes it is. @milanholemans and I crossed streams. Like we agreed, we closed his PR in favor of this one.

@milanholemans milanholemans self-assigned this Jun 2, 2026

@milanholemans milanholemans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks promising, would like to see this in action. Made a few remarks while reviewing the skill.

Comment thread .github/skills/pr-review/references/pr-checklist.md Outdated
Comment thread .github/skills/pr-review/references/pr-checklist.md Outdated
Comment thread .github/skills/pr-review/references/pr-checklist.md
Comment thread .github/skills/pr-review/references/pr-checklist.md Outdated
Comment thread .github/skills/pr-review/references/pr-checklist.md Outdated
Comment thread .github/skills/pr-review/SKILL.md
Comment thread .github/skills/pr-review/SKILL.md
Comment thread .github/skills/pr-review/SKILL.md
Comment thread .github/skills/pr-review/SKILL.md Outdated
Comment thread .github/skills/pr-review/SKILL.md
@milanholemans
milanholemans marked this pull request as draft June 7, 2026 20:53
@waldekmastykarz
waldekmastykarz marked this pull request as ready for review June 22, 2026 12:14
waldekmastykarz and others added 8 commits July 8, 2026 09:09
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Milan Holemans <11723921+milanholemans@users.noreply.github.com>
Co-authored-by: Milan Holemans <11723921+milanholemans@users.noreply.github.com>
- Remove obsolete PII/telemetry tracking rule (handled centrally by Zod)
- Replace 'All tests must pass' with 'npm test must pass without errors'
- Add Documentation Quality section (D) to SKILL.md
- Add Reference Commands section pointing to example implementations

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace optionSet rule with custom Zod validation (option sets obsolete)
- Add test checks: command name/description not null, schema validation
- Add sinon.restore() in after() rule for tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove MkDocs warning check (not applicable to Docusaurus)
- Add command page sections check to documentation
- Remove logger.logToStderr from SKILL.md (enforced by ESLint)
- Remove telemetry mention from SKILL.md (handled centrally by Zod)
- Add verbose logging requirement to SKILL.md
- Clarify defaultProperties is for list commands only
- Specify formatting utilities for escaping user input

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
waldekmastykarz and others added 2 commits July 14, 2026 09:05
ESLint already handles this validation through a custom rule,
so it's redundant for the AI reviewer to check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The 'include the marker to incorporate global options' line was a
MkDocs convention. Docusaurus uses component imports instead.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 31699654-6aea-43f7-bb59-89b2e5adca0c

@milanholemans milanholemans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from a little change. Also let's not forget to update our docs checklist as well.

Comment thread .github/skills/pr-review/SKILL.md Outdated

Post inline comments and the summary as a single review submission. Always target the actual line where the issue exists.

**API limitation**: The GitHub API can only target lines within diff hunk ranges (changed lines + surrounding context lines). If the target line falls in a gap between hunks, do **not** post it on a nearby line. Instead, include that finding in the summary (the review body) with the file path and line number. Only use `suggestion` blocks for findings posted on the exact target line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@milanholemans
milanholemans merged commit 8f7febb into pnp:main Jul 15, 2026
10 checks passed
@milanholemans milanholemans added this to the v11.10 milestone Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants