Skip to content

Clean up Code Review Skill#129152

Draft
mrsharm wants to merge 1 commit into
dotnet:mainfrom
mrsharm:musharm/ImprovingCR
Draft

Clean up Code Review Skill#129152
mrsharm wants to merge 1 commit into
dotnet:mainfrom
mrsharm:musharm/ImprovingCR

Conversation

@mrsharm

@mrsharm mrsharm commented Jun 9, 2026

Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings June 9, 2026 03:09
@github-actions github-actions Bot added the area-skills Agent Skills label Jun 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restructures the code-review skill documentation by moving the large rule lists out of SKILL.md into focused reference markdown files, and updates the skill text to direct reviewers to load the relevant references on demand.

Changes:

  • Expanded the code-review skill’s frontmatter description to better describe when to use it and what it covers.
  • Updated Step 4 to explicitly instruct loading the appropriate rule reference(s) before flagging issues.
  • Extracted the detailed rule sections into separate references/*.md files and added a “Rule Reference Index” table linking to them.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/skills/code-review/SKILL.md Updates the skill description, adds “load references first” guidance, and replaces embedded rule blocks with a “Rule Reference Index” pointing to new reference docs.
.github/skills/code-review/references/testing.md New: testing/regression-test conventions extracted into a dedicated reference.
.github/skills/code-review/references/platform-and-native.md New: cross-platform, native/interop, and VM/runtime patterns reference.
.github/skills/code-review/references/performance.md New: performance and allocation guidance reference.
.github/skills/code-review/references/documentation.md New: documentation/commenting and breaking-change guidance reference.
.github/skills/code-review/references/correctness-and-safety.md New: correctness, safety, thread-safety, and security guidance reference.
.github/skills/code-review/references/codebase-consistency.md New: PR hygiene, deduplication, and codebase consistency guidance reference.
.github/skills/code-review/references/code-style.md New: code style/formatting guidance reference.
.github/skills/code-review/references/api-design.md New: API design/approval/contract guidance reference.

- **Add static asserts for hardcoded structural offsets.** When using hardcoded offsets to access struct fields (especially in assembly), add static asserts to verify them.
- **Use minipal for new platform abstractions.** Use minipal (new) instead of PAL (legacy) for platform abstraction in new CoreCLR code. Use `ALTERNATE_ENTRY` (not `LOCAL_LABEL`) for assembly labels called from outside their function.
- **Use `JITDUMP` and `LOG` macros, not `printf`.** In JIT code use `JITDUMP`. In CoreCLR VM use `LOG()`/`LOGGING` defines. Do not use `printf` or `Console.WriteLine` in production native code.
The detailed dotnet/runtime review conventions live in reference files under [`references/`](./references/). They were extracted from 43,000+ maintainer review comments across 6,600+ PRs and represent the standards enforced in practice. **During [Step 4](#step-4-detailed-analysis), load only the reference file(s) relevant to what the PR changes** — most reviews need two or three. When in doubt, load more rather than fewer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-skills Agent Skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants