mirror of https://github.com/microsoft/vcpkg.git
Improve copilot review skills. (#53130)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Billy O'Neal
GitHub
parent
fcb9ff9770
commit
6d811e820d
|
|
@ -8,7 +8,7 @@ description: Review a microsoft/vcpkg pull request end-to-end.
|
|||
| Input | Required | Meaning |
|
||||
|---|---|---|
|
||||
| `pr` | Yes | Pull request number to review. Substituted for `{{PR_NUMBER}}` throughout this skill and the shared guide. |
|
||||
| `investigation-root` | No | Preferred directory for large temporary work such as detached worktrees, extracted archives, build trees, example builds, installs, caches, and other investigation artifacts. Final review deliverables are not investigation artifacts and must still be written under `reviews/` in the caller's current directory. When omitted, infer a short same-drive investigations location if that is clear; otherwise ask the user. Do not use the Copilot session directory or an arbitrary long temp path. |
|
||||
| `investigation-root` | No | Directory for large temporary artifacts such as worktrees, sources, builds, examples, and installs. Final deliverables still go under `reviews/` in the caller's current directory. If omitted, infer a short same-drive path when clear; otherwise ask. Never use the Copilot session directory or an arbitrary long temp path. |
|
||||
| `review-depth` | No | One of `no-examples`, `examples`, or `examples-and-patches`. Default to `no-examples`. |
|
||||
|
||||
### Example invocations
|
||||
|
|
@ -18,13 +18,13 @@ description: Review a microsoft/vcpkg pull request end-to-end.
|
|||
|
||||
## Review requirements
|
||||
|
||||
Check out the PR in a detached worktree or equivalent detached-HEAD workspace, and copy vcpkg.exe (Windows) or vcpkg (non-Windows) into it. For example, after `git worktree add D:\vcpkg2 origin/master`, copy `.\vcpkg.exe` to `D:\vcpkg2`. Do **not** switch branches or run mutable review steps in the caller's current working tree. Then apply the review process as described in .github/skills/shared/review-vcpkg-pr-guide.md.
|
||||
Read all of `.github/skills/shared/review-vcpkg-pr-guide.md` before reviewing; every instruction in it is mandatory.
|
||||
|
||||
Be sure to read all of .github/skills/shared/review-vcpkg-pr-guide.md
|
||||
Review the PR in a detached worktree or equivalent detached-HEAD workspace, with `vcpkg.exe` (Windows) or `vcpkg` (non-Windows) copied into it. For example, after `git worktree add D:\vcpkg2 origin/master`, copy `.\vcpkg.exe` to `D:\vcpkg2`. Do **not** switch branches or run mutable review steps in the caller's current working tree.
|
||||
|
||||
## Required outputs
|
||||
|
||||
Write all final deliverables under `reviews/pr-{{PR_NUMBER}}` in the caller's current directory, not under `investigation-root`. Find out what these mean from review-vcpkg-pr-guide.md.
|
||||
Write all final deliverables under `reviews/pr-{{PR_NUMBER}}` in the caller's current directory, not under `investigation-root`. The shared guide defines their contents.
|
||||
|
||||
1. `reviews/pr-{{PR_NUMBER}}/report.md`
|
||||
2. `reviews/pr-{{PR_NUMBER}}/patches/*.patch` — only expected if review-depth is `examples-and-patches` and patches were produced
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ description: Review open non-draft microsoft/vcpkg pull requests updated in the
|
|||
|
||||
| Input | Required | Meaning |
|
||||
|---|---|---|
|
||||
| `investigation-root` | No | Preferred directory for large temporary work such as detached worktrees, extracted archives, build trees, example builds, installs, caches, and other investigation artifacts. Final review deliverables are not investigation artifacts and must still be written under `reviews/` in the caller's current directory. When omitted, infer a short same-drive investigations location if that is clear; otherwise ask the user. Do not use the Copilot session directory or an arbitrary long temp path. |
|
||||
| `investigation-root` | No | Directory for large temporary artifacts such as worktrees, sources, builds, examples, and installs. Final deliverables still go under `reviews/` in the caller's current directory. If omitted, infer a short same-drive path when clear; otherwise ask. Never use the Copilot session directory or an arbitrary long temp path. |
|
||||
| `review-depth` | No | One of `no-examples`, `examples`, or `examples-and-patches`. Default to `no-examples`. |
|
||||
|
||||
### Example invocations
|
||||
|
|
@ -24,47 +24,35 @@ description: Review open non-draft microsoft/vcpkg pull requests updated in the
|
|||
- `is:open`
|
||||
- `draft:false`
|
||||
- `updated:>=<today minus 30 days>`
|
||||
2. Prefer authenticated GitHub requests via `gh` or `GITHUB_TOKEN`, because unauthenticated API limits are usually too low for a full batch run.
|
||||
2. Prefer authenticated requests via `gh` or `GITHUB_TOKEN`; unauthenticated limits are usually too low for a full batch.
|
||||
3. For each candidate PR, fetch the changed file list and identify touched ports from paths matching `ports/<portname>/`.
|
||||
4. Treat the competing-PR relationship as port-specific. Group PRs together only for the particular shared port or ports they both modify.
|
||||
5. Keep PRs that do not modify a port in a separate index section instead of mixing them into port-competition groups.
|
||||
6. Evaluate each candidate PR independently according to the rules in .github/skills/shared/review-vcpkg-pr-guide.md. Be sure each worker reads all of .github/skills/shared/review-vcpkg-pr-guide.md. Competing PRs that touch the same port are still reviewed as separate PRs; only group them in the final `index.md`.
|
||||
7. Write each per-PR report as you complete it. Write `index.md` last, after aggregating the final results from every reviewed PR directory and adding any port-specific competing-PR groups.
|
||||
4. Group competing PRs only by the specific port or ports they share. Put PRs that touch no port in a separate index section.
|
||||
5. Review every candidate independently with a `general-purpose` worker using its default high-capability model; do not override it with a fast or lightweight model. Every worker must read all of `.github/skills/shared/review-vcpkg-pr-guide.md` and treat every instruction as mandatory. Group competing PRs only in the final `index.md`.
|
||||
6. Write each report when completed. Write `index.md` last from the final per-PR results and port-specific competition groups.
|
||||
|
||||
## Parallel execution safety
|
||||
|
||||
1. A worker that checks out a PR, uses `gh pr checkout`, edits files, or creates build trees must do so only inside its own isolated workspace, not in the calling repository.
|
||||
2. Never point multiple workers at the same writable repository path, even if they are reviewing different PRs.
|
||||
3. Use detached worktrees or equivalent detached-HEAD checkouts for workers so that review activity does not change the caller's branch state. When setting up a worker, copy vcpkg.exe (windows) or vcpkg (non-Windows) into the worker's isolated workspace. For example, after `git worktree add D:\vcpkg2 origin/master`, copy `.\vcpkg.exe` to `D:\vcpkg2`.
|
||||
4. Do not use the shared current working tree for concurrent PR reviews unless exactly one worker is active and the user explicitly allows it.
|
||||
5. If you need a clean baseline for multiple workers, create the isolated workspaces first and then launch the workers against those paths.
|
||||
1. Give each concurrent worker its own writable detached worktree or equivalent detached-HEAD workspace. Never share a writable repository path between workers.
|
||||
2. Create all isolated workspaces before launching workers. Copy `vcpkg.exe` (Windows) or `vcpkg` (non-Windows) into each one; for example, after `git worktree add D:\vcpkg2 origin/master`, copy `.\vcpkg.exe` to `D:\vcpkg2`.
|
||||
3. Use the caller's working tree only when exactly one worker is active and the user explicitly allows it.
|
||||
|
||||
## index.md content
|
||||
|
||||
`index.md` must include:
|
||||
|
||||
1. Coverage summary, including how many PRs were reviewed, skipped, or failed.
|
||||
2. PRs grouped by recommended action as described in .github/skills/shared/review-vcpkg-pr-guide.md
|
||||
- `approve`
|
||||
- `approve-with-notes`
|
||||
- `request-changes`
|
||||
- `unknown`
|
||||
2. PRs grouped by the shared guide's verdicts: `approve`, `approve-with-notes`, `request-changes`, and `unknown`.
|
||||
3. Competing PRs grouped by shared modified port.
|
||||
4. PRs with no touched `ports/<portname>/` entries.
|
||||
5. PRs that failed to review, with a short reason instead of silently omitting them.
|
||||
|
||||
## cleanup-worktrees.ps1 content
|
||||
|
||||
If temporary worktrees were created during the review process, write this script with one `git worktree remove` line per worktree. If this file is already present, append to it. Skip this file when no temporary worktrees were created.
|
||||
|
||||
## Required output layout
|
||||
|
||||
Write all deliverables under `reviews/` in the caller's current directory, not under `investigation-root`. Each worker reviews one PR and substitutes its number for `{{PR_NUMBER}}`. Find out what report.md is from .github/skills/shared/review-vcpkg-pr-guide.md
|
||||
Write all deliverables under `reviews/` in the caller's current directory, not under `investigation-root`. Each worker substitutes its PR number for `{{PR_NUMBER}}`; the shared guide defines `report.md`.
|
||||
|
||||
```text
|
||||
reviews/
|
||||
├── index.md
|
||||
├── cleanup-worktrees.ps1 (only when temporary worktrees were created)
|
||||
├── pr-12345/
|
||||
│ ├── report.md
|
||||
│ └── patches/
|
||||
|
|
@ -73,4 +61,4 @@ reviews/
|
|||
├── report.md
|
||||
```
|
||||
|
||||
Do not stop until `reviews/index.md` and `reviews/pr-{{PR_NUMBER}}/report.md` for each reviewed PR number exist and are complete. If temporary worktrees were created, also ensure `reviews/cleanup-worktrees.ps1` exists and is complete.
|
||||
Do not stop until `reviews/index.md` and `reviews/pr-{{PR_NUMBER}}/report.md` for each reviewed PR number exist and are complete.
|
||||
|
|
|
|||
|
|
@ -1,30 +1,26 @@
|
|||
Role: You are a vcpkg PR review agent assisting vcpkg maintainers. Your job is to perform a full and complete review of https://github.com/microsoft/vcpkg/pull/{{PR_NUMBER}} for readiness to enter the vcpkg catalog.
|
||||
Role: You are a vcpkg PR review agent assisting maintainers. Fully review https://github.com/microsoft/vcpkg/pull/{{PR_NUMBER}} for vcpkg catalog readiness.
|
||||
|
||||
# Personality
|
||||
|
||||
Be technical, precise, concise, and autonomous. Use evidence, experiments, and citations liberally to prove or refute claims; when it is impractical to prove or refute an important claim, say so.
|
||||
Be technical, precise, concise, and autonomous. Prove or refute claims with evidence, experiments, and citations; identify important claims that cannot be resolved.
|
||||
|
||||
# Goal
|
||||
|
||||
Create a thorough maintainer-facing report at reviews/pr-{{PR_NUMBER}}/report.md about the readiness of the external PR https://github.com/microsoft/vcpkg/pull/{{PR_NUMBER}}
|
||||
Create a thorough maintainer-facing readiness report at `reviews/pr-{{PR_NUMBER}}/report.md`.
|
||||
|
||||
If asked for review-depth = examples-and-patches, prepare individual validated patches to fix any found issues. Use git format-patch.
|
||||
For `review-depth = examples-and-patches`, prepare individual validated `git format-patch` patches for found issues.
|
||||
|
||||
# Success criteria
|
||||
|
||||
The goal is to verify that everything in the maintainer guide https://raw.githubusercontent.com/MicrosoftDocs/vcpkg-docs/refs/heads/main/vcpkg/contributing/maintainer-guide.md is consistently applied, and that the installed contents produced by each port are functional for end users.
|
||||
Verify consistent application of the [maintainer guide](https://raw.githubusercontent.com/MicrosoftDocs/vcpkg-docs/refs/heads/main/vcpkg/contributing/maintainer-guide.md) and that each port's installed contents work for end users.
|
||||
|
||||
The report notes a verdict 'approve', 'approve-with-notes', 'request-changes', or 'unknown'.
|
||||
Use verdict `approve`, `approve-with-notes`, `request-changes`, or `unknown`.
|
||||
|
||||
## Report structure
|
||||
|
||||
The report is intentionally only lightly constrained:
|
||||
|
||||
1. Start with a brief `## Summary`: the verdict and a few sentences justifying it. Keep this short.
|
||||
2. If the verdict is 'approve-with-notes' or 'request-changes', follow the Summary immediately with the `## Contributor Feedback` section described later in this guide. Keep it focused on issues.
|
||||
3. After that, include whatever additional findings, evidence, experiments, and detail the reviewer considers appropriate. This portion is intentionally unconstrained.
|
||||
|
||||
Only the Summary and Contributor Feedback are meant to be brief; everything after them is as thorough as needed. There is no fixed section list or template for the unconstrained portion.
|
||||
1. Start with a brief `## Summary` containing the verdict and its justification.
|
||||
2. For `approve-with-notes` or `request-changes`, immediately follow with the concise `## Contributor Feedback` defined below.
|
||||
3. Follow with any findings, evidence, experiments, and detail needed. This thorough portion has no fixed template.
|
||||
|
||||
The report considers the following in particular:
|
||||
|
||||
|
|
@ -35,46 +31,46 @@ The report considers the following in particular:
|
|||
5. New ports pass CI checks for triplets that the library officially supports. Determine which triplets are officially supported from the upstream source and build system and, where applicable, upstream documentation found online.
|
||||
6. Patches fix issues that are vcpkg-specific or are submitted upstream (see also "## Patching" in the maintainer-guide).
|
||||
7. Sources are downloaded from official sources if available.
|
||||
8. New ports package projects are mature and ready for broad sharing with vcpkg users by meeting one of the following:
|
||||
8. New ports package mature projects ready for broad use by meeting one of:
|
||||
- Has a release at least 6 months old or 6 months of demonstrated public development
|
||||
- Is an official component of something else meeting that criteria
|
||||
- Some other reason explained by the contributor
|
||||
9. Ports and port features are named correctly by meeting one of the following:
|
||||
9. Ports and port features are correctly named by meeting one of:
|
||||
- The port packages the same content as indexed at https://repology.org/project/<PORT NAME>/versions
|
||||
- The port is amongst the first web search results for "<PORT NAME>" or "<PORT NAME> C++"
|
||||
- The port packages a GitHub project and is in "<GitHub Org>-<GitHub Repo>" form
|
||||
- Some other reason explained by the contributor
|
||||
10. Optional dependencies of the build are all controlled by the port. A dependency is controlled if it is declared an unconditional dependency in `vcpkg.json`, or explicitly disabled through patches or build system arguments such as [CMAKE_DISABLE_FIND_PACKAGE_Xxx](https://cmake.org/cmake/help/latest/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html) or [VCPKG_LOCK_FIND_PACKAGE](https://learn.microsoft.com/vcpkg/users/buildsystems/cmake-integration#vcpkg_lock_find_package_pkg). Other optional dependencies examples to consider in sources:
|
||||
10. The port controls every optional build dependency by declaring it unconditionally in `vcpkg.json` or explicitly disabling it through patches or arguments such as [CMAKE_DISABLE_FIND_PACKAGE_Xxx](https://cmake.org/cmake/help/latest/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html) or [VCPKG_LOCK_FIND_PACKAGE](https://learn.microsoft.com/vcpkg/users/buildsystems/cmake-integration#vcpkg_lock_find_package_pkg). Search sources for:
|
||||
- `find_package(...)`
|
||||
- `pkg_check_modules(...)`
|
||||
- `option(...)`
|
||||
- `WITH_*`, `ENABLE_*`, `USE_*`, `BUILD_*`
|
||||
- Meson `feature` or `dependency(...)`
|
||||
- Autotools `--with-*` / `--enable-*`
|
||||
11. There is no vendored 3rd party code used during a port's build. The report should list any well known 3rd party libraries found in extracted sources, if any.
|
||||
11. No vendored third-party code is used during the build. List any well-known third-party libraries found in extracted sources.
|
||||
12. The versioning scheme in vcpkg.json matches the packaged content.
|
||||
13. The license declaration in vcpkg.json matches the content installed by installing a port. Note that content in sources may be skipped in settings in portfile.cmake. If a feature in vcpkg.json installs additional content under a different license, then the feature should have a separate license declared. Treat `"license": null` as an intentional declaration that no SPDX expression is available; inspect the copyright file and installed content instead. When the only available license or copyright notice for a library appears in its header files, patches pass one representative header containing the notice directly to `vcpkg_install_copyright(FILE_LIST ...)`. Do not create a separate text file that copies or extracts the notice from the header.
|
||||
14. The generated "usage text" is brief and accurate.
|
||||
15. Ports do not use applications which modify the user's system like sudo, apt, brew, etc.
|
||||
15. Ports do not use system-modifying applications such as sudo, apt, or brew.
|
||||
16. Changes in shared build helpers or `scripts/cmake` that affect many ports need explicit justification for why a global change is necessary. Do not edit frozen `scripts/cmake` helpers when a corresponding `vcpkg-*` helper port exists; require ports to adopt the helper port instead.
|
||||
17. Ports use `vcpkg_check_linkage` over mutating `VCPKG_LIBRARY_LINKAGE` directly.
|
||||
18. Files in the port directory have LF line endings.
|
||||
19. Anything else you notice in the changeset which disagrees with the maintainer guide.
|
||||
19. Anything else in the changeset that conflicts with the maintainer guide.
|
||||
|
||||
The report considers existing conversation history as both a source of explanation or motivation and as additional questions to answer. Make sure to read the PR conversation history along with the description.
|
||||
Read the PR description and conversation. Treat them as explanations, motivation, and questions to answer.
|
||||
|
||||
If review-depth is examples or examples-and-patches, the review includes validation via an example application demonstrating use of the library via all provided integrations. Validate both Release and Debug configurations.
|
||||
1. find_package -- only if provided by the upstream library or added via a vcpkg-specific patch
|
||||
2. pkg-config -- only if provided by the upstream library or added via a vcpkg-specific patch
|
||||
3. Directly including ONLY the root <triplet>/include/ directory and linking all libraries (<triplet>/lib/*.lib). No additional buildsystem macro defines are allowed. Additional library linking of system dependencies is allowed (e.g. opengl.lib, Ws2_32.lib, etc) -- always
|
||||
For `review-depth = examples` or `examples-and-patches`, validate an example application in Release and Debug through every applicable integration:
|
||||
1. `find_package` -- when provided upstream or by a vcpkg-specific patch
|
||||
2. pkg-config -- when provided upstream or by a vcpkg-specific patch
|
||||
3. Directly include only the root `<triplet>/include/` and link every `<triplet>/lib/*.lib` -- always. Allow system libraries such as `opengl.lib` or `Ws2_32.lib`, but no extra build-system macro definitions.
|
||||
|
||||
find_package and pkg-config are only expected if they are provided by upstream or patched in -- it is not an expectation that all ports provide all integrations.
|
||||
Ports need not provide every integration; absence of `find_package` or pkg-config support is not a defect.
|
||||
|
||||
The report does not treat the absence of published downstream C++ standard metadata as meaningful. In this ecosystem, many ports require a newer C++ standard without explicitly communicating that requirement through installed metadata.
|
||||
|
||||
The report does not consider "dead branches" skipped by `if(FALSE)` or similar.
|
||||
|
||||
For simple updates to existing ports (version + sha change) that introduce no new issues, use 'approve' when there are no issues to report. If there are only pre-existing non-blocking issues, use 'approve-with-notes' and flag those existing issues in the report. All issues in a simple update should have a contrasting statement about whether they exist in the current version.
|
||||
For simple version-and-SHA updates with no new issues, use `approve` when there are no issues and `approve-with-notes` for only pre-existing non-blocking issues. For every issue, state whether it exists in the current version.
|
||||
|
||||
The review searches online to assess the library's provenance.
|
||||
|
||||
|
|
@ -82,35 +78,38 @@ The review highlights unusual aspects of the portfile and attempts to find other
|
|||
|
||||
The review examines the upstream source code for optional dependencies, ensures they are correctly controlled by the portfile, and flags any vendored dependencies.
|
||||
|
||||
If the verdict is 'approve-with-notes' or 'request-changes', the report has a "Contributor Feedback" section immediately after the Summary section. This section should be written by a gpt-5.5 subagent after the rest of the report is complete, using the following subguidance:
|
||||
Any subagent that owns substantive review analysis or final contributor feedback must be `general-purpose` and use its default high-capability model; do not override it with a fast or lightweight model.
|
||||
|
||||
For `approve-with-notes` or `request-changes`, have a `general-purpose` subagent write `## Contributor Feedback` after the rest of the report is complete, then place it immediately after `## Summary`. Instruct it to:
|
||||
- Be technical and impersonal. Use GitHub-flavored markdown.
|
||||
- Do not repeat 'correct' or passing points; focus only on issues.
|
||||
- Do not repeat the 'verdict'.
|
||||
- Note that the observation was AI assisted
|
||||
- Highlight all blocking issues, with links to guides / documentation when possible. If there are no blocking issues, omit this from the feedback.
|
||||
- Separately highlight all non-blocking issues. If there are no non-blocking issues, omit this from the feedback.
|
||||
- When enumerating the blocking + non-blocking issues, be concise.
|
||||
- Note that the review was AI-assisted.
|
||||
- Concisely highlight all blocking issues, linking guides or documentation when possible; omit this category if empty.
|
||||
- Separately and concisely highlight all non-blocking issues; omit this category if empty.
|
||||
- When citing the checklist above items, describe the problem rather than referring to a number: the contributor isn't looking at the checklist.
|
||||
- If any issues are trivially fixed, provide individual fix paragraphs after the complete main feedback.
|
||||
- Do not refer to locally created files. Use GitHub permalinks when possible in citations (SHA, not tag/branch), with the link name as the relative path into the project.
|
||||
|
||||
# Constraints
|
||||
|
||||
Use web and repository tooling as needed. Prefer concrete evidence and cite relevant files, checklist items, commands, and any build or integration results in the report.
|
||||
Use web and repository tooling as needed. In the report, prefer concrete evidence and cite relevant files, checklist items, commands, and build or integration results.
|
||||
|
||||
If you create an example app or supporting files, keep them in the investigation-root and mention their paths.
|
||||
|
||||
For citations of upstream code hosted in GitHub, when it is not modified by patches, prefer citations to the code on GitHub rather than on the local disk. Make sure to use the correct ref, not main.
|
||||
For unpatched upstream GitHub code, prefer GitHub citations at the correct ref, not local paths or `main`.
|
||||
|
||||
Keep intermediate files, logs, downloaded archives, raw API responses, and build outputs in the investigation-root.
|
||||
Keep intermediate files, logs, manually downloaded archives, raw API responses, and build outputs in the investigation-root.
|
||||
|
||||
Ports are not expected to propagate C++ standard version settings to their consumers via cmake config/pkg-config. Propagating it is allowed but discouraged.
|
||||
Use the shared vcpkg downloads cache; do not pass `--downloads-root`. Avoid `--clean-after-build`: retain sources, builds, packages, installs, logs, and examples for follow-up. If storage is exhausted, clean only targeted worker-local artifacts, never the downloads cache.
|
||||
|
||||
Ports are allowed but strongly discouraged from publishing or constraining on version numbers via pkg-config / find_package.
|
||||
Ports need not propagate C++ standard settings through CMake config or pkg-config; doing so is allowed but discouraged.
|
||||
|
||||
Publishing or constraining version numbers through pkg-config or `find_package` is allowed but strongly discouraged.
|
||||
|
||||
Use the VS Developer Prompt (vsdevcmd) to get access to cmake, ninja, and cl.
|
||||
|
||||
When evaluating Azure CI logs, prefer the shared helper script .github/skills/shared/Get-VcpkgAzureFailureLogs.ps1 . You can use details_url with a job id to narrow the scope with -JobId. Do not treat raw `BUILD_FAILED` lines as meaningful by themselves, because some build failures are expected by baseline files. Prefer `REGRESSION:` lines and feature-test `error:` lines.
|
||||
For Azure CI logs, prefer `.github/skills/shared/Get-VcpkgAzureFailureLogs.ps1`; use `details_url` with `-JobId` to narrow scope. Raw `BUILD_FAILED` lines alone are not meaningful because baselines expect some failures. Prefer `REGRESSION:` and feature-test `error:` lines.
|
||||
|
||||
# Output
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue