pre-pr-self-review

스타1

포크0

업데이트2026년 7월 10일 23:07

Self-review checklist - plus an agentic review pass (a read-only reviewer persona over the diff) - before opening a PR. Use before invoking `create-pr`, when reviewing your own draft, or when a reviewer flags issues that should have been caught earlier. Codifies recurring mistakes from multi-targeted polyfill work - missing tests for new public surface, unchecked length sums, null-pointer foot-guns from `MemoryMarshal.GetReference` on empty spans, drift from `ArgumentNullException.ThrowIfNull` and `checked()` conventions, TFM phrasing errors, and stale PR descriptions.

설치

Codex 또는 Claude로 설치 이 Prompt를 복사해 Codex, Claude 또는 다른 어시스턴트에 붙여 넣으면 Skill 페이지를 검토하고 설치를 진행할 수 있습니다.

출처

JeremyKuhne

JeremyKuhne/agent-skills

GitHub 저장소 열기 Creator 저장소 보기

다운로드

관련 직업SOC

SOC 직업 분류 기준

소프트웨어 개발자컴퓨터 및 수학직·SOC 15-1252

SKILL.md

readonly

이 저장소의 다른 Skills

같은 저장소

cswin32-com

JeremyKuhne/agent-skills

Guides struct-based COM interop using CsWin32 - AOT-compatible raw pointer calls without [ComImport] or built-in CLR marshalling; managed [ComImport] mirrors are limited to CCW bridges. Consult when working with ComScope, IID.Get, delegate* unmanaged vtables, CoCreateInstance or a class factory, IComIID across .NET 10 and .NET Framework, connection-point Advise/Unadvise ownership, caller-owned CCW references, manually defining COM interfaces absent from Win32 metadata, COM pointer lifetime in fields, cross-assembly CCWs, or mocking struct-based COM in tests. Not for ordinary RCW-based COM consumption with no raw struct pointers or CCW bridge. Paired with the cswin32-interop skill for the general P/Invoke layer and blittable-signature rules.

2026-07-201

cswin32-interop

JeremyKuhne/agent-skills

Guides CsWin32 P/Invoke interop in a multi-targeted .NET 10 and .NET Framework library. Consult when replacing [DllImport] with source-generated PInvoke.* calls, working with generated Windows.Win32 projections (HANDLE / HMODULE / HRESULT / BOOL and typed enum/constant types), configuring NativeMethods.txt / NativeMethods.json, composing a public PInvoke across foundation/owner/extender packages with extensionReceiver, deciding which support library owns a helper, auditing native allocation and byte/element lengths, selecting compile-time / runtime / analyzer guards for Windows-only code, or choosing between CsWin32 and [LibraryImport]. Not for managed-only changes with no native boundary. Paired with the cswin32-com skill for the struct-based COM layer.

2026-07-201

performance-testing

JeremyKuhne/agent-skills

Author and run BenchmarkDotNet performance tests in a multi-targeted .NET library's perf project, and translate a user's outcome-shaped performance question into a measurement. Use when adding new benchmarks, running existing ones, comparing implementations, profiling to find which method or source line dominates, evaluating allocations / memory usage, reading the generated code (sharplab / DisassemblyDiagnoser / HardwareCounters), or when a user asks how long something takes, how much memory it uses, where time is spent, or to help make a method faster - which this skill turns into a scenario, a benchmark, and a drill-down.

2026-07-191

engineering-baseline

JeremyKuhne/agent-skills

Create a new .NET repository fully wired for building, testing, and publishing with OSS best practices, or bring an existing repository up to a documented engineering baseline. Use when asked to "create a new repository" / "scaffold a new project" (a CLI tool, a class library / NuGet package, or a multi-target library) with CI, packaging, branch protection, and governance files, or to "ensure this repo follows modern engineering best practices" / "audit this repository" / "bring this repo up to standard". Scores a repository across nine domains - foundation, build, test, publish, versioning, CI, supply-chain security, OSS governance, and agent enablement - against an industry-cited baseline, then reports gaps and remediates. Irreversible or remote actions (repository creation, branch protection, release settings, publishing) are proposed for explicit approval, never run silently.

2026-07-111

github-actions-cost-optimization

JeremyKuhne/agent-skills

Audit and reduce GitHub Actions runner usage and normalized cost without weakening required validation. Use when asked to reduce CI cost, spend, minutes, or job count; optimize workflow triggers, matrices, runner selection, caches, or artifact retention; eliminate duplicate Actions runs; or decide which checks should be automatic versus scheduled or manual. Not for application runtime performance, which belongs in a performance-testing workflow.

2026-07-111

framework-jit-optimization

JeremyKuhne/agent-skills

Optimize hot-path code for the `net481` (.NET Framework) target in a multi-targeted library's Framework-only sources. Use when writing or reviewing performance-sensitive loops, deciding whether to specialize a generic method for primitive types, choosing between scalar/unrolled/BCL-delegating implementations, or diagnosing why a net481 micro-benchmark regresses on the older RyuJIT. Also covers the modern .NET (net10) counterpart - vectorization, hardware intrinsics, struct-generic kernels, JIT-friendly shapes - and the cross-TFM codegen fundamentals shared by both targets (arithmetic and branchless lowering, struct layout, zero-allocation static data, hot-path allocation anti-patterns). For BenchmarkDotNet harness mechanics (authoring/running benchmarks, evaluating allocations) see the `performance-testing` skill.

2026-07-101

name	pre-pr-self-review
description	Self-review checklist - plus an agentic review pass (a read-only reviewer persona over the diff) - before opening a PR. Use before invoking `create-pr`, when reviewing your own draft, or when a reviewer flags issues that should have been caught earlier. Codifies recurring mistakes from multi-targeted polyfill work - missing tests for new public surface, unchecked length sums, null-pointer foot-guns from `MemoryMarshal.GetReference` on empty spans, drift from `ArgumentNullException.ThrowIfNull` and `checked()` conventions, TFM phrasing errors, and stale PR descriptions.
license	MIT
compatibility	Requires git plus the repository's build, test, and agent-file validation commands.
metadata	{"portability":"portable","applicability":"universal","binding":"optional-overlay","risk":"local-write","maturity":"canary","requires":"none","related":"create-pr, address-pr-feedback, security-review, performance-testing"}

Pre-PR self-review

If overlay.md exists beside this file, read it before acting; it contains repository-specific bindings. This core remains usable without it.

Run this checklist before invoking the create-pr skill. Each item is a question your code or PR body must answer. Update the skill whenever a reviewer flags something not yet listed.

This skill pairs with several others a consuming repo wires concretely in its overlay: a polyfill-dotnet-api skill (the source-preference and design rules this checklist validates), create-pr (the workflow this precedes), address-pr-feedback (the follow-up that re-runs this checklist), performance-testing (benchmark authoring required when a perf claim drives a change), framework-jit-optimization (net481 RyuJIT tradeoffs cited in the polyfill-correctness items), agent-files-review (for changes under .agents/, AGENTS.md, or .github/copilot-instructions.md), and security-review (the security-specific subset - abusive-input handling, length / integer overflow, allocation and algorithmic DoS, argument validation, and every use of unsafe / Unsafe.* / MemoryMarshal.* / Marshal.* or any BCL API whose docs say "unsafe" or "caller must"). Invoke security-review alongside this checklist for any change that adds or modifies a member accepting caller-supplied data, or that touches one of those caller-validated constructs - the common case, not a niche.

Agentic review pass

Before walking the checklist, run an automated review pass over the diff so the reviewer-bot class of findings - correctness edge cases, hand-rolled parser/format pitfalls, CI and supply-chain hygiene, and doc-vs-behavior drift - surfaces locally instead of across PR rounds. Spawn a read-only pre-PR reviewer persona (a consuming repo wires the concrete agent in its overlay) over the working diff, triage its findings (valid / nit / judgment call / likely false positive), fix the valid ones, and re-run at most once when the fixes were non-trivial. Keep it bounded: a same-class model nitpicks indefinitely, so two passes is the cap and the deterministic gates - tests, lint, the format validators - stay the source of truth. The checklist below is the human complement: the recurring, domain-specific mistakes an agent pass tends to miss.

1. Tests cover every new branch

For each new public (or InternalsVisibleTo-internal) member:

Search the test projects for the symbol; no hits = missing test.
Polyfills in the Framework-only tree: tests run on both TFMs. Wrap polyfill-only paths (subclass fallbacks, null-receiver guards) in #if NETFRAMEWORK.
Runtime type-check fast paths (typeof(T) == obj.GetType()): test the fast path and a subclass override.
Generic primitive specializations: every specialized branch needs a test. Don't rely on byte/int covering bool/sbyte/short/ ushort/uint/long/ulong - each has independent ref reinterpretation.
Security-sensitive APIs (FixedTimeEquals, hex decode): cover equal, differing-content, length mismatch, both empty, one empty, and a long span where only the last byte differs.
Allocating APIs (Concat, ToHexString): include an OverflowException test on the length sum.

Test hygiene for the tests themselves - the recurring miss list that costs the most review rounds on coverage-only PRs:

Test method names start with the method under test. MethodName_StateUnderTest_ExpectedBehavior per the repo's test conventions. ReadOnlySpan_Empty_ReturnsEmpty is wrong; SliceAtNull_ReadOnlySpan_Empty_ReturnsEmpty is right.
Every IDisposable test local uses using or try/finally. A temp-folder helper, a matcher handle, a pooled-list rental - a bare local leaks the resource when an assertion fails. Use the try/finally pattern when the test itself exercises explicit Dispose().
Don't hard-code InvariantCulture for APIs that use CurrentCulture. Provider-less formatting helpers generally format with CurrentCulture. Asserting against InvariantCulture-formatted strings makes the test locale-dependent.

2. Polyfill / framework correctness

For any change in the Framework-only tree (a polyfill or a framework-only fast path), walk these items (a consuming repo may keep the per-item detail and code patterns in a polyfill-correctness overlay companion):

Empty / null spans handled before unsafe interop (empty source, empty destination, both empty; exception type cross-checked).
Multi-input length sums wrapped in checked().
Throw helpers use the standard BCL exceptions, not custom types.
Span overloads stay allocation-free by default (document any trade-off in <remarks>).
Behavior parity with the modern BCL (exception type and message family, edge cases, type-exact fast paths).
Performance claims name the JIT (net481 RyuJIT vs modern .NET RyuJIT) and are measured or explicitly marked unmeasured.

If the change is not in the Framework-only tree, skip to section 3.

3. PR description matches reality

TFM phrasing: name the polyfill's target TFM (the framework target, e.g. net472) distinctly from the TFM the tests merely run on (e.g. net481). Do not call a net472-targeted polyfill "net481-only".
File list, test counts, and perf numbers all reflect the current diff. Re-run after every commit; do not paste numbers from an earlier iteration.
Walk each bullet of the description against the diff before pushing. If the body says "covers Foo with cases A/B/C", search the diff for tests named Foo_… and confirm A, B, and C are all there. Review rounds have been lost to descriptions claiming a case (a Span<char> "null at end", a double-dispose test) that was not actually in the diff.
"Deliberately deferred" entries match what's actually absent from the working tree.

4. Final audit before staging

git status --short - delete leftover probe / scratch files; confirm every listed file belongs in the change set.
git diff --check - whitespace.
Rebase onto the canonical main if the branch trails it. Use upstream/main when working from a fork (the canonical repo lives at upstream), origin/main when cloning the canonical repo directly. Recently-merged sister PRs may have introduced files this PR cross-references; running off a stale base point makes those links look broken to an offline link check that gates .agents/**, AGENTS.md, and *.instructions.md changes. A PR once lost a review round to exactly this.
For changes that touch .agents/, AGENTS.md, or .github/copilot-instructions.md, also run the repo's agent-file link checker (see the agent-files-review skill for options, including changed-only and base-ref modes).
Build both TFMs.
Run the test suite in both Debug and Release. Release-mode RyuJIT inlining surfaces bugs Debug doesn't - e.g. [AggressiveInlining] + Unsafe.As<T, byte>(ref param) propagates the caller's int-promoted argument into the comparison immediate (cmp ecx, 0xFFFFFFFF instead of cmp ecx, 0xFF) for negative signed-primitive inputs on net481, but only in Release. Mask explicitly with & 0xFF / & 0xFFFF. See the polyfill-dotnet-api and framework-jit-optimization skills.
Stage by path, never git add -A / git add . when the working tree spans more than one logical change. If topics are intermingled, ask before staging.

5. Failing CI is a stop, not a sprint

When a build / test / CI run fails on a PR:

Diagnose.
Prepare the fix in the working tree.
Describe what changed, why, and any risks.
Stop and wait for explicit approval before commit/push.

Stacked rapid-fire fix commits are how perf regressions and unrelated sweep-ups get into history.

6. Update this skill

If a reviewer flags something not in this checklist, add it. If the review touched .agents/ files, also update via the agent-files-review workflow so the validator and any CI mirror stay in sync.