|
| 1 | +--- |
| 2 | +name: pull-requests |
| 3 | +description: > |
| 4 | + Guidance for creating pull requests and handling PR review comments in the |
| 5 | + Agent Framework repository. Use this when writing a PR description (filling out |
| 6 | + the PR template) or when responding to and resolving review comments on an |
| 7 | + existing PR. |
| 8 | +--- |
| 9 | + |
| 10 | +# Pull Request Workflow |
| 11 | + |
| 12 | +This skill covers two tasks: (1) writing a high-quality PR description, and |
| 13 | +(2) handling review comments on an existing PR. |
| 14 | + |
| 15 | +## 1. Writing the PR description |
| 16 | + |
| 17 | +Always follow the repository PR template at |
| 18 | +[`.github/pull_request_template.md`](../../pull_request_template.md). Keep its |
| 19 | +exact structure and headings. Fill every section: |
| 20 | + |
| 21 | +### `### Motivation & Context` |
| 22 | +Explain *why* the change is needed: the problem it solves and the scenario it |
| 23 | +contributes to. Describe the net change relative to `main` — this is implied, so |
| 24 | +do **not** spell out "vs main" explicitly. |
| 25 | + |
| 26 | +### `### Description & Review Guide` |
| 27 | +Describe the changes, the overall approach, and the design. Answer the three |
| 28 | +prompts: |
| 29 | +- **What are the major changes?** |
| 30 | +- **What is the impact of these changes?** |
| 31 | +- **What do you want reviewers to focus on?** — This item is for **human |
| 32 | + reviewers only**. Automated/AI reviewers must ignore it and review the entire |
| 33 | + change rather than narrowing scope to it. |
| 34 | + |
| 35 | +### `### Related Issue` |
| 36 | +Link the issue the PR fixes using a GitHub closing keyword (`Fixes #123` / |
| 37 | +`Closes #123`) so it closes automatically on merge. A PR with no linked issue may |
| 38 | +be closed regardless of how valid the change is. Before opening, confirm there is |
| 39 | +no other open PR for the same issue; if there is, explain how this PR differs. |
| 40 | + |
| 41 | +### `### Contribution Checklist` |
| 42 | +Check every item that applies. For the breaking-change item: |
| 43 | +- Leave **"This is not a breaking change."** checked for the common case. |
| 44 | +- If the change **is** breaking, add the `breaking change` label **or** put a |
| 45 | + `[BREAKING]` prefix in the title — a workflow keeps the label and the title |
| 46 | + prefix in sync automatically (see `.github/workflows/label-title-prefix.yml` |
| 47 | + and `.github/workflows/label-breaking-change.yml`). |
| 48 | + |
| 49 | +### Do not |
| 50 | +- Do **not** add ad-hoc sections such as "Validation" or "Tests run"; CI/CD and |
| 51 | + the checklist already cover validation status. |
| 52 | +- Do **not** remove or reorder the template's headings. |
| 53 | + |
| 54 | +### Creating the PR |
| 55 | +Open new PRs as **drafts** until they are ready for review. Example: |
| 56 | + |
| 57 | +```bash |
| 58 | +gh pr create --repo microsoft/agent-framework --base main \ |
| 59 | + --head <your-fork-owner>:<branch> --draft \ |
| 60 | + --title "<concise title>" --body "<body following the template>" |
| 61 | +``` |
| 62 | + |
| 63 | +## 2. Handling review comments |
| 64 | + |
| 65 | +When a PR receives review comments, follow this sequence — **do not start editing |
| 66 | +code before the user has reviewed the plan**: |
| 67 | + |
| 68 | +1. **Review the comments.** Read every review comment and thread on the PR, |
| 69 | + including inline code comments and general review summaries. |
| 70 | +2. **Make a plan.** Produce a concrete plan describing how each comment will be |
| 71 | + addressed (or why it should not be, with reasoning). |
| 72 | +3. **Let the user review the plan.** Present the plan and wait for the user's |
| 73 | + approval or adjustments before implementing anything. |
| 74 | +4. **Implement.** Make the agreed changes. |
| 75 | +5. **Reply to every comment.** Add a reply to **all** comments explaining how it |
| 76 | + was addressed (or the agreed outcome) — leave none unanswered. |
| 77 | +6. **Resolve resolved threads.** Mark a review thread as resolved only when the |
| 78 | + comment has actually been addressed. |
| 79 | + |
| 80 | +### Useful commands |
| 81 | + |
| 82 | +List review comments and threads: |
| 83 | + |
| 84 | +```bash |
| 85 | +# Inline review comments |
| 86 | +gh api repos/{owner}/{repo}/pulls/{pr}/comments |
| 87 | + |
| 88 | +# Review threads with resolution state (GraphQL) |
| 89 | +gh api graphql -f query=' |
| 90 | + query($owner:String!,$repo:String!,$pr:Int!){ |
| 91 | + repository(owner:$owner,name:$repo){ |
| 92 | + pullRequest(number:$pr){ |
| 93 | + reviewThreads(first:100){ |
| 94 | + nodes{ id isResolved comments(first:50){ nodes{ id body author{login} } } } |
| 95 | + } |
| 96 | + } |
| 97 | + } |
| 98 | + }' -F owner={owner} -F repo={repo} -F pr={pr} |
| 99 | +``` |
| 100 | + |
| 101 | +Reply to an inline review comment: |
| 102 | + |
| 103 | +```bash |
| 104 | +gh api repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies \ |
| 105 | + -f body="Addressed in <commit>: <explanation>" |
| 106 | +``` |
| 107 | + |
| 108 | +Resolve a review thread (needs the thread node id from the GraphQL query above): |
| 109 | + |
| 110 | +```bash |
| 111 | +gh api graphql -f query=' |
| 112 | + mutation($threadId:ID!){ |
| 113 | + resolveReviewThread(input:{threadId:$threadId}){ thread{ isResolved } } |
| 114 | + }' -F threadId={thread_id} |
| 115 | +``` |
0 commit comments