chore: fix Pi prompt template argument syntax (#6543)
- Fix @1 -> $1 in landpr.md - Fix $@ -> $1 in reviewpr.md - Remove stray /reviewpr line from reviewpr.md - Delete old pr.md (replaced by reviewpr.md and landpr.md)main
Christian Klotz
GitHub
parent
6c03fe1a4d
commit
395810a60b
|
|
@ -0,0 +1,105 @@
|
|||
---
|
||||
description: Land a PR (merge with proper workflow)
|
||||
---
|
||||
|
||||
Input
|
||||
|
||||
- PR: $1 <number|url>
|
||||
- If missing: use the most recent PR mentioned in the conversation.
|
||||
- If ambiguous: ask.
|
||||
|
||||
Do (review-only)
|
||||
Goal: produce a thorough review and a clear recommendation (READY for /landpr vs NEEDS WORK). Do NOT merge, do NOT push, do NOT make changes in the repo as part of this command.
|
||||
|
||||
1. Identify PR meta + context
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json number,title,state,isDraft,author,baseRefName,headRefName,headRepository,url,body,labels,assignees,reviewRequests,files,additions,deletions --jq '{number,title,url,state,isDraft,author:.author.login,base:.baseRefName,head:.headRefName,headRepo:.headRepository.nameWithOwner,additions,deletions,files:.files|length}'
|
||||
```
|
||||
|
||||
2. Read the PR description carefully
|
||||
- Summarize the stated goal, scope, and any “why now?” rationale.
|
||||
- Call out any missing context: motivation, alternatives considered, rollout/compat notes, risk.
|
||||
|
||||
3. Read the diff thoroughly (prefer full diff)
|
||||
|
||||
```sh
|
||||
gh pr diff <PR>
|
||||
# If you need more surrounding context for files:
|
||||
gh pr checkout <PR> # optional; still review-only
|
||||
git show --stat
|
||||
```
|
||||
|
||||
4. Validate the change is needed / valuable
|
||||
- What user/customer/dev pain does this solve?
|
||||
- Is this change the smallest reasonable fix?
|
||||
- Are we introducing complexity for marginal benefit?
|
||||
- Are we changing behavior/contract in a way that needs docs or a release note?
|
||||
|
||||
5. Evaluate implementation quality + optimality
|
||||
- Correctness: edge cases, error handling, null/undefined, concurrency, ordering.
|
||||
- Design: is the abstraction/architecture appropriate or over/under-engineered?
|
||||
- Performance: hot paths, allocations, queries, network, N+1s, caching.
|
||||
- Security/privacy: authz/authn, input validation, secrets, logging PII.
|
||||
- Backwards compatibility: public APIs, config, migrations.
|
||||
- Style consistency: formatting, naming, patterns used elsewhere.
|
||||
|
||||
6. Tests & verification
|
||||
- Identify what’s covered by tests (unit/integration/e2e).
|
||||
- Are there regression tests for the bug fixed / scenario added?
|
||||
- Missing tests? Call out exact cases that should be added.
|
||||
- If tests are present, do they actually assert the important behavior (not just snapshots / happy path)?
|
||||
|
||||
7. Follow-up refactors / cleanup suggestions
|
||||
- Any code that should be simplified before merge?
|
||||
- Any TODOs that should be tickets vs addressed now?
|
||||
- Any deprecations, docs, types, or lint rules we should adjust?
|
||||
|
||||
8. Key questions to answer explicitly
|
||||
- Can we fix everything ourselves in a follow-up, or does the contributor need to update this PR?
|
||||
- Any blocking concerns (must-fix before merge)?
|
||||
- Is this PR ready to land, or does it need work?
|
||||
|
||||
9. Output (structured)
|
||||
Produce a review with these sections:
|
||||
|
||||
A) TL;DR recommendation
|
||||
|
||||
- One of: READY FOR /landpr | NEEDS WORK | NEEDS DISCUSSION
|
||||
- 1–3 sentence rationale.
|
||||
|
||||
B) What changed
|
||||
|
||||
- Brief bullet summary of the diff/behavioral changes.
|
||||
|
||||
C) What’s good
|
||||
|
||||
- Bullets: correctness, simplicity, tests, docs, ergonomics, etc.
|
||||
|
||||
D) Concerns / questions (actionable)
|
||||
|
||||
- Numbered list.
|
||||
- Mark each item as:
|
||||
- BLOCKER (must fix before merge)
|
||||
- IMPORTANT (should fix before merge)
|
||||
- NIT (optional)
|
||||
- For each: point to the file/area and propose a concrete fix or alternative.
|
||||
|
||||
E) Tests
|
||||
|
||||
- What exists.
|
||||
- What’s missing (specific scenarios).
|
||||
|
||||
F) Follow-ups (optional)
|
||||
|
||||
- Non-blocking refactors/tickets to open later.
|
||||
|
||||
G) Suggested PR comment (optional)
|
||||
|
||||
- Offer: “Want me to draft a PR comment to the author?”
|
||||
- If yes, provide a ready-to-paste comment summarizing the above, with clear asks.
|
||||
|
||||
Rules / Guardrails
|
||||
|
||||
- Review only: do not merge (`gh pr merge`), do not push branches, do not edit code.
|
||||
- If you need clarification, ask questions rather than guessing.
|
||||
|
|
@ -1,36 +0,0 @@
|
|||
---
|
||||
description: Review PRs from URLs with structured issue and code analysis
|
||||
---
|
||||
|
||||
You are given one or more GitHub PR URLs: $@
|
||||
|
||||
For each PR URL, do the following in order:
|
||||
|
||||
1. Read the PR page in full. Include description, all comments, all commits, and all changed files.
|
||||
2. Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
|
||||
3. Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
|
||||
4. Check if docs/\*.md require modification. This is usually the case when existing features have been changed, or new features have been added.
|
||||
5. Provide a structured review with these sections:
|
||||
- Good: solid choices or improvements
|
||||
- Bad: concrete issues, regressions, missing tests, or risks
|
||||
- Ugly: subtle or high impact problems
|
||||
6. Add Questions or Assumptions if anything is unclear.
|
||||
7. Add Change summary and Tests.
|
||||
|
||||
Output format per PR:
|
||||
PR: <url>
|
||||
Good:
|
||||
|
||||
- ...
|
||||
Bad:
|
||||
- ...
|
||||
Ugly:
|
||||
- ...
|
||||
Questions or Assumptions:
|
||||
- ...
|
||||
Change summary:
|
||||
- ...
|
||||
Tests:
|
||||
- ...
|
||||
|
||||
If no issues are found, say so under Bad and Ugly.
|
||||
|
|
@ -0,0 +1,105 @@
|
|||
---
|
||||
description: Review a PR thoroughly without merging
|
||||
---
|
||||
|
||||
Input
|
||||
|
||||
- PR: $1 <number|url>
|
||||
- If missing: use the most recent PR mentioned in the conversation.
|
||||
- If ambiguous: ask.
|
||||
|
||||
Do (review-only)
|
||||
Goal: produce a thorough review and a clear recommendation (READY for /landpr vs NEEDS WORK). Do NOT merge, do NOT push, do NOT make changes in the repo as part of this command.
|
||||
|
||||
1. Identify PR meta + context
|
||||
|
||||
```sh
|
||||
gh pr view <PR> --json number,title,state,isDraft,author,baseRefName,headRefName,headRepository,url,body,labels,assignees,reviewRequests,files,additions,deletions --jq '{number,title,url,state,isDraft,author:.author.login,base:.baseRefName,head:.headRefName,headRepo:.headRepository.nameWithOwner,additions,deletions,files:.files|length}'
|
||||
```
|
||||
|
||||
2. Read the PR description carefully
|
||||
- Summarize the stated goal, scope, and any "why now?" rationale.
|
||||
- Call out any missing context: motivation, alternatives considered, rollout/compat notes, risk.
|
||||
|
||||
3. Read the diff thoroughly (prefer full diff)
|
||||
|
||||
```sh
|
||||
gh pr diff <PR>
|
||||
# If you need more surrounding context for files:
|
||||
gh pr checkout <PR> # optional; still review-only
|
||||
git show --stat
|
||||
```
|
||||
|
||||
4. Validate the change is needed / valuable
|
||||
- What user/customer/dev pain does this solve?
|
||||
- Is this change the smallest reasonable fix?
|
||||
- Are we introducing complexity for marginal benefit?
|
||||
- Are we changing behavior/contract in a way that needs docs or a release note?
|
||||
|
||||
5. Evaluate implementation quality + optimality
|
||||
- Correctness: edge cases, error handling, null/undefined, concurrency, ordering.
|
||||
- Design: is the abstraction/architecture appropriate or over/under-engineered?
|
||||
- Performance: hot paths, allocations, queries, network, N+1s, caching.
|
||||
- Security/privacy: authz/authn, input validation, secrets, logging PII.
|
||||
- Backwards compatibility: public APIs, config, migrations.
|
||||
- Style consistency: formatting, naming, patterns used elsewhere.
|
||||
|
||||
6. Tests & verification
|
||||
- Identify what's covered by tests (unit/integration/e2e).
|
||||
- Are there regression tests for the bug fixed / scenario added?
|
||||
- Missing tests? Call out exact cases that should be added.
|
||||
- If tests are present, do they actually assert the important behavior (not just snapshots / happy path)?
|
||||
|
||||
7. Follow-up refactors / cleanup suggestions
|
||||
- Any code that should be simplified before merge?
|
||||
- Any TODOs that should be tickets vs addressed now?
|
||||
- Any deprecations, docs, types, or lint rules we should adjust?
|
||||
|
||||
8. Key questions to answer explicitly
|
||||
- Can we fix everything ourselves in a follow-up, or does the contributor need to update this PR?
|
||||
- Any blocking concerns (must-fix before merge)?
|
||||
- Is this PR ready to land, or does it need work?
|
||||
|
||||
9. Output (structured)
|
||||
Produce a review with these sections:
|
||||
|
||||
A) TL;DR recommendation
|
||||
|
||||
- One of: READY FOR /landpr | NEEDS WORK | NEEDS DISCUSSION
|
||||
- 1–3 sentence rationale.
|
||||
|
||||
B) What changed
|
||||
|
||||
- Brief bullet summary of the diff/behavioral changes.
|
||||
|
||||
C) What's good
|
||||
|
||||
- Bullets: correctness, simplicity, tests, docs, ergonomics, etc.
|
||||
|
||||
D) Concerns / questions (actionable)
|
||||
|
||||
- Numbered list.
|
||||
- Mark each item as:
|
||||
- BLOCKER (must fix before merge)
|
||||
- IMPORTANT (should fix before merge)
|
||||
- NIT (optional)
|
||||
- For each: point to the file/area and propose a concrete fix or alternative.
|
||||
|
||||
E) Tests
|
||||
|
||||
- What exists.
|
||||
- What's missing (specific scenarios).
|
||||
|
||||
F) Follow-ups (optional)
|
||||
|
||||
- Non-blocking refactors/tickets to open later.
|
||||
|
||||
G) Suggested PR comment (optional)
|
||||
|
||||
- Offer: "Want me to draft a PR comment to the author?"
|
||||
- If yes, provide a ready-to-paste comment summarizing the above, with clear asks.
|
||||
|
||||
Rules / Guardrails
|
||||
|
||||
- Review only: do not merge (`gh pr merge`), do not push branches, do not edit code.
|
||||
- If you need clarification, ask questions rather than guessing.
|
||||
Loading…
Reference in New Issue