Skip to content

Change copilot code review from instruciton to skill - #1718

Open
Steve Lee (SteveL-MSFT) wants to merge 2 commits into
PowerShell:mainfrom
SteveL-MSFT:copilot-code-review
Open

Steve Lee (SteveL-MSFT) wants to merge 2 commits into
PowerShell:mainfrom
SteveL-MSFT:copilot-code-review

Conversation

@SteveL-MSFT

Copy link
Copy Markdown
Member

PR Summary

Based on GitHub, it appears that it's better to have code review as a skill than an instruction so moved and updated the existing code review instructions as a skill.

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.

🔵 Needs a closer look

The Windows routing/docs and applyTo patterns should be aligned (notably around resources/WindowsUpdate/ and resources/windows_*) to ensure the intended guidance actually applies.

Pull request overview

This PR introduces a new Copilot Skill for repository code review by organizing review guidance into area-specific instruction files under .github/skills/code-review/, and updating the skill’s routing documentation accordingly.

Changes:

  • Added area-specific code review guidance files (engine, resources, adapters, extensions, CLI, tests, libraries, performance, security, Windows, Linux/macOS).
  • Updated .github/skills/code-review/SKILL.md to reference the new .github/skills/code-review/ instruction locations and provide path-based routing.
  • Added Windows/test/security-focused review guidance to better target sensitive/OS-specific code paths.
File summaries
File Description
.github/skills/code-review/SKILL.md Skill entry point; documents area routing for applying the right review guidance per path.
.github/skills/code-review/engine.instructions.md Engine-focused guidance (settings precedence, discovery, caching, what-if behavior).
.github/skills/code-review/resource.instructions.md Resource-focused guidance (manifest/schema coherence, operation consistency, error handling).
.github/skills/code-review/adapter.instructions.md Adapter-focused guidance (single-mode operation, structured I/O, PS-specific pitfalls).
.github/skills/code-review/extension.instructions.md Extension-focused guidance (discovery protocol, schema updates, PS parallel behavior).
.github/skills/code-review/cli.instructions.md CLI-focused guidance (output determinism, server mode parity, compatibility).
.github/skills/code-review/tests.instructions.md Test-focused guidance (cross-platform correctness, isolation/cleanup, assertions).
.github/skills/code-review/library.instructions.md Library-focused guidance (API visibility, error handling, serde/schema care).
.github/skills/code-review/performance.instructions.md Rust performance guidance (caching correctness, allocation patterns, interop cleanup).
.github/skills/code-review/security.instructions.md Security-sensitive guidance (fail-closed behavior, ACL nuances, secrets handling).
.github/skills/code-review/windows.instructions.md Windows-specific guidance (FFI/COM safety, ACL semantics, service/firewall edge cases).
.github/skills/code-review/linux.instructions.md Linux/macOS-specific guidance (path handling, platform gating, permission checks).
Review details

Suppressed comments (1)

.github/skills/code-review/SKILL.md:27

  • The Windows routing row doesn't include resources/WindowsUpdate/, but the repo uses that exact (capitalized) directory name and you reference it in windows.instructions.md. Adding it here keeps the routing documentation consistent and prevents confusion when looking for Windows guidance applicability.
  • Files reviewed: 1/12 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

🔵 Needs a closer look

Missing skill metadata and invalid supporting-file paths prevent the guidance from loading correctly.

Review details

Suppressed comments (2)

.github/skills/code-review/SKILL.md:18

  • These instruction-file paths are not relative to SKILL.md; they resolve to a nonexistent nested skills/code-review directory. Use ./<file> references for supporting skill files so Copilot can load the routed guidance. The same correction is needed for every row in this table.
    .github/skills/code-review/SKILL.md:11
  • The directory still will not be discovered as an agent skill: SKILL.md uses instruction-file metadata (applyTo) and omits the required name field. Add the skill name matching the directory; otherwise this PR's review guidance remains unavailable as a skill.

This issue also appears on line 18 of the same file.

  • Files reviewed: 1/12 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants