Skip to content

Add code review instructions derived from PR feedback patterns#1646

Draft
SteveL-MSFT wants to merge 2 commits into
mainfrom
stevel-msft-add-code-review-instructions
Draft

Add code review instructions derived from PR feedback patterns#1646
SteveL-MSFT wants to merge 2 commits into
mainfrom
stevel-msft-add-code-review-instructions

Conversation

@SteveL-MSFT

Copy link
Copy Markdown
Member

Copilot's automated code reviews sometimes flag false positives (e.g., claiming code won't compile when it does) or miss repo-specific patterns that human reviewers consistently catch. This PR adds a .github/instructions/code-review.instructions.md file to teach Copilot the recurring patterns and best practices specific to this repository.

Approach

Analyzed review comments across 30+ merged PRs spanning Dec 2025 -- Jul 2026, focusing on:

  • Human reviewer feedback that corrected Copilot or identified issues Copilot missed
  • Patterns where Copilot's feedback was accepted and led to fixes
  • Recurring themes in maintainer comments

The resulting instructions file is organized by domain and covers:

  • General principles -- avoid false compile-error claims, distinguish test harnesses from production code, recognize auto-generated files
  • Rust patterns -- cache key correctness, API visibility, fail-closed security, Windows FFI/COM safety (DLL loading, Drop, HRESULT), what-if correctness, Option return types
  • Pester test patterns -- cross-platform paths, env var isolation, ACL cleanup, exit code assertions, array comparisons, event subscriber cleanup, cmdlet skip guards
  • PowerShell adapter patterns -- module array returns, $using: in parallel blocks, terminating vs non-terminating errors
  • Design/architecture -- settings precedence chain, backward compatibility, semver conventions, canonical property naming, schema coherence
  • CI/CD -- fork permission limitations, conditional tool installation
  • Documentation -- debug print removal, locale string accuracy, dead i18n keys

Notes

  • The applyTo frontmatter targets **/*.rs, **/*.ps1, **/*.psm1, **/*.tests.ps1, **/*.json to cover the main file types in this repo
  • Instructions are phrased as actionable review guidance, not general coding advice
  • The "General Principles" section specifically addresses known false-positive patterns to reduce noise in future reviews

Analyzed review comments across 30+ PRs spanning Dec 2025 - Jul 2026 to extract
recurring patterns and repo-specific best practices for code reviews:

- General: avoid false-positive compile claims, respect test harness vs production
  distinction, recognize auto-generated files
- Rust: cache keys, API visibility, fail-closed security, Windows FFI/COM safety,
  DLL loading security, what-if correctness, Option return types, schema coherence
- Pester tests: cross-platform paths, env var isolation, ACL cleanup, exit code
  assertions, array comparisons, event subscriber cleanup, cmdlet skip guards
- PowerShell adapters: module array returns, $using: in parallel blocks,
  terminating vs non-terminating error distinction
- Design: settings precedence, backward compatibility, semver for resources,
  canonical property naming conventions, noFiltering semantics
- CI/CD: fork permissions, conditional tool install, PS naming conventions
- Docs: remove debug prints, locale string accuracy, dead i18n keys

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 23, 2026 23:12

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 adds a repository-specific GitHub Copilot code review instruction set under .github/instructions/ to reduce recurring false positives and encode patterns maintainers commonly enforce across DSC’s Rust, PowerShell/Pester, and JSON changes.

Changes:

  • Introduces a new Copilot code-review instruction file with guidance grouped by domain (Rust, PowerShell/Pester, architecture, CI/CD, docs/logging).
  • Adds targeted “avoid noise” principles (e.g., CI-validated compile/test claims, test-harness vs production expectations, autogenerated file handling).

Comment thread .github/instructions/code-review.instructions.md
Comment thread .github/instructions/code-review.instructions.md Outdated
@github-actions

Copy link
Copy Markdown

Code Coverage Report

No Rust files were changed in this PR.

🔵 Full Codebase Coverage

81% (good)

Metric Value
Total executable lines 18188
Lines covered by tests 14885
Coverage percentage 81%

Full codebase coverage measures all instrumented Rust lines across the project.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 00:52

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@SteveL-MSFT
SteveL-MSFT marked this pull request as draft July 24, 2026 21:14
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