# Pr Review > Reviews a NuGet/NuGet.Client pull request like a senior maintainer — reviews in an isolated workspace by default, or against the reviewer's existing clone (self-review / local changes), builds and tests the affected projects, traces changes across files, and returns high-signal, severity-tagged findings with a merge verdict. Invoke explicitly with a PR number or branch. - Skill: `nuget/pr-review` (Agent Skill) - Install (CLI): `npx skillmds@latest add nuget/pr-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/nuget/pr-review/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: nuget (https://skillmd.com/u/nuget) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/nuget/pr-review --- # NuGet/NuGet.Client PR Review Review pull requests the way NuGet/NuGet.Client's senior maintainers do. **Requires shell access** (`git`/`dotnet`). Work in four phases: **Set up, Understand and verify, Judge, and Report.** --- ## Set up Pick the mode that matches whose change you're reviewing. ### Mode A — someone else's PR (default): isolated workspace Use this when the change is on a branch or PR you don't have locally, so your `git`/`dotnet` commands never disturb anyone's active checkout. 1. **Workspace path:** `%LOCALAPPDATA%\GitHubCopilot\ReviewSkill\NuGet.Client` — reused across reviews, so the clone is a one-time cost. 2. **Clone or update:** clone if missing, else `git -C {workspace} fetch origin`. 3. **Get the PR:** `git -C {workspace} fetch origin pull/{PR}/head:pr-{PR}` then `git -C {workspace} checkout pr-{PR}`; for a branch, `git -C {workspace} fetch origin {branch}` then check it out. 4. **Restore when done:** `git -C {workspace} checkout -`. ### Mode B — your own or a local change: review the existing clone Use this when the reviewer is the author, already has the branch in their NuGet.Client clone, or wants to review uncommitted local edits before committing. - **Skip the clone and checkout** — work in the reviewer's existing NuGet.Client clone (use the current working directory if it is one, otherwise ask for the path). - **Do not switch branches or touch the working tree** — review whatever is currently checked out, including uncommitted work. - **Pick the diff to review:** uncommitted (`git diff`), staged (`git diff --staged`), or the current branch against its base — ask which if it's ambiguous. Default to Mode A when given a PR number or a branch you don't have locally; default to Mode B when invoked with no argument, with `local`/`current`, or when the reviewer says they're reviewing their own or in-progress change. ### Build and debug — read the repo's own runbooks, don't guess - Build/test/debug (incl. cross-platform): read **`docs/debugging.md`**, **`docs/cross-platform-debugging.md`**, and **`CONTRIBUTING.md`**. - **Build and test ONLY the affected project(s), never the whole solution** — a full NuGet.Client build/test is very slow. Build the changed project and run just the test project(s) that cover the change. --- ## Understand and verify 1. **Scope by area** and read the matching repo guideline before reviewing those files: | Changed area | Read before reviewing | |---|---| | `src/NuGet.Core/**` restore/resolution/protocol | `docs/coding-guidelines.md` | | `src/NuGet.Clients/**` (VS/WPF) | `docs/ui-guidelines.md`, `docs/coding-guidelines.md` | | user-facing strings / `*.resx` | `docs/localizability.md` | | API / type design | `docs/design-review-guide.md` | | `test/**` | `docs/coding-guidelines.md` (test conventions) | 2. **Trace beyond the diff — mandatory for any behavior/correctness finding.** The bug usually lives in how a *caller* consumes the changed value, not in the diff. For every changed method whose **return value, branch, or side effect changes**: - Open its direct **callers and callees, across project/assembly boundaries** — a value that looks benign at the change site can violate an assumption a consumer in another assembly relies on. - Enumerate the behavioral cases the change can take and state which path each hits — the distinct input shapes, config/environment states, and success vs. empty/failure results the code can encounter. - A new `return null` / empty / short-circuit / early-exit is **not reviewed** until you have read what every consumer does with it. 3. **Verify, don't guess.** Run the affected test(s) or reproduce the path; otherwise cite the exact code path that proves the finding. **Never assert the *direction* of a behavior change from the diff alone.** If you can't confirm it, say so and flag it — do not clear a concern as a non-issue without tracing it. --- ## Judge (review priorities) Hold each change to the guideline doc for its area (the *Understand and verify* table) — check the written rule, don't rely on memory. Surface findings in this priority order: 1. **Correctness and tests** (most frequent) — a behavior change must ship a test that *fails without the fix*; verify boundaries and every consumer of a new early-exit (see *Understand and verify*). 2. **Performance** 3. **API surface** — treat public API / `PublicAPI.*.txt` as a permanent commitment; otherwise per `design-review-guide.md`. 4. **Conventions** — rules per `coding-guidelines.md`. 5. **VS/IDE** (`src/NuGet.Clients/**`) — per `ui-guidelines.md`. 6. **Localization / telemetry / maintainability** — per `localizability.md` and `coding-guidelines.md`. 7. **Process hygiene** — focused scope; PR template with a linked `Fixes:` issue; tracking issue for deferred work. ### Severity - **[blocking]** — correctness bugs; a missing or ineffective test for a behavior change; or an unverified behavior change on a hot path (defaults to **[blocking]** until a test pins the behavior or the author confirms intent). - **[suggestion]** — Need to be addressed before merge, but not a correctness bug. Examples: performance, API surface, conventions, VS/IDE, localization, telemetry, maintainability, or process hygiene. - **[nit]** — does not need to be addressed before merge, but would improve the PR. Examples: style, naming, comments, or minor refactoring. --- ## Report For each finding: ``` [severity] path/to/File.cs:Line — Why: Suggest: ``` Then a **verdict**: - **Request changes** if any [blocking] finding exists, **or if an unresolved behavior-change question remains open** (never "Approve" while a behavior change is unverified). - **Approve with suggestions** if only [suggestion]/[nit]. - **Approve** if clean. Be concise, cite `file:line`, give an actionable fix for every finding, and state which projects you built and which tests you ran.