Skip to content

docs: overhaul Copilot instructions with coding conventions and prompt files#40111

Closed
benhillis wants to merge 1 commit intomasterfrom
user/benhill/copilot_updates
Closed

docs: overhaul Copilot instructions with coding conventions and prompt files#40111
benhillis wants to merge 1 commit intomasterfrom
user/benhill/copilot_updates

Conversation

@benhillis
Copy link
Copy Markdown
Member

Summary of the Pull Request

Overhauls the Copilot instruction files so that AI-assisted code generation and review follows the actual conventions used in this repo.

PR Checklist

  • Closes: N/A
  • Communication: N/A (documentation-only change)
  • Tests: N/A (no code changes)
  • Localization: N/A
  • Dev docs: This PR is the dev docs update
  • Documentation updated: N/A

Detailed Description of the Pull Request / Additional comments

Updated .github/copilot-instructions.md:

  • Added coding conventions section: naming (m_camelCase members, PascalCase() methods, c_camelCase constants), WIL error handling, RAII/smart pointers, synchronization (wil::srwlock + _Guarded_by_), strings (std::wstring, std::format), headers (#pragma once, precomp.h), localization, telemetry, IDL/COM ABI rules, config file conventions
  • Added namespace-to-directory map for top-level namespaces
  • Added key source files list (defs.h, WslTelemetry.h, wslc.idl, etc.)
  • Replaced clang-format references with .\FormatSource.ps1
  • Consolidated duplicate timing info into single reference table
  • Fixed Unix-only find command that wouldn't work on Windows

New files:

  • .github/copilot/review.md - Review prompt focused on high-risk areas (ABI breaks, missing localization, resource safety)
  • .github/copilot/test.md - Test generation prompt with TAEF patterns, macros, skip helpers
  • .github/copilot/commit.md - Commit message guidelines
  • .editorconfig - Editor settings for non-C++ files (YAML, Python, XML/resw, etc.)

Validation Steps Performed

Manual review of all conventions against actual codebase patterns.

…t files

Major update to .github/copilot-instructions.md:
- Add coding conventions (naming, error handling, RAII, strings, headers,
  synchronization, localization, telemetry, formatting, IDL/COM, config)
- Add test authoring summary pointing to detailed test.md prompt
- Add namespace-to-directory map for top-level namespaces
- Add key source files list (defs.h, WslTelemetry.h, wslc.idl, etc.)
- Replace clang-format references with .\FormatSource.ps1
- Consolidate duplicate timing info into single reference table

New files:
- .github/copilot/review.md: Review prompt focused on high-risk areas
  (ABI breaks, missing localization, resource safety)
- .github/copilot/test.md: Test generation prompt with TAEF patterns
- .github/copilot/commit.md: Commit message guidelines
- .editorconfig: Editor settings for non-C++ files

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the repository’s Copilot guidance to better align AI-assisted code generation/review with WSL’s conventions, and adds dedicated prompt files plus an EditorConfig for non-C++ files.

Changes:

  • Expanded .github/copilot-instructions.md with repo-specific coding conventions, navigation hints, and updated validation/testing guidance.
  • Added dedicated Copilot prompt files for review, test generation, and commit messaging under .github/copilot/.
  • Introduced a new .editorconfig intended to complement .clang-format for non-C++ files.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
.github/copilot/test.md Adds TAEF test-generation patterns and macro guidance.
.github/copilot/review.md Adds review focus areas (ABI, localization, resource safety, etc.).
.github/copilot/commit.md Provides commit message guidance.
.github/copilot-instructions.md Major expansion of repo-specific conventions and workflows.
.editorconfig New editor settings for indentation, line endings, and whitespace across file types.

Comment thread .github/copilot/review.md
@@ -0,0 +1,23 @@
## Code Review Guidelines for WSL

When reviewing code, enforce the conventions in `copilot-instructions.md`. Focus especially on these high-risk areas:
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

review.md references copilot-instructions.md, but the actual repo file is .github/copilot-instructions.md. This will make the prompt point reviewers at a non-existent document; please update the reference (and include the relative path if needed).

Suggested change
When reviewing code, enforce the conventions in `copilot-instructions.md`. Focus especially on these high-risk areas:
When reviewing code, enforce the conventions in `.github/copilot-instructions.md`. Focus especially on these high-risk areas:

Copilot uses AI. Check for mistakes.
Comment thread .github/copilot/test.md
Comment on lines +6 to +9
Tests use TAEF (Test Authoring and Execution Framework). Always include `"Common.h"`.

### Test Class Structure
```cpp
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The test snippet and guidance say to always include "Common.h", but existing Windows tests include "precomp.h" first and then "Common.h" (e.g., test/windows/*.cpp). The example should reflect the required include order so generated tests compile in the test project.

Suggested change
Tests use TAEF (Test Authoring and Execution Framework). Always include `"Common.h"`.
### Test Class Structure
```cpp
Tests use TAEF (Test Authoring and Execution Framework). Always include `"precomp.h"` first, then `"Common.h"`.
### Test Class Structure
```cpp
#include "precomp.h"

Copilot uses AI. Check for mistakes.
Comment thread .github/copilot/test.md
Comment on lines +43 to +52
### Assertion Macros
- `VERIFY_ARE_EQUAL(expected, actual)` — value equality
- `VERIFY_ARE_NOT_EQUAL(a, b)` — value inequality
- `VERIFY_IS_TRUE(condition)` — boolean check
- `VERIFY_IS_FALSE(condition)` — negative boolean check
- `VERIFY_IS_NULL(ptr)` — null check
- `VERIFY_IS_NOT_NULL(ptr)` — non-null check
- `VERIFY_WIN32_BOOL_SUCCEEDED(expr)` — Win32 BOOL result
- `VERIFY_SUCCEEDED(hr)` — HRESULT success

Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The doc states VERIFY_ARE_EQUAL(expected, actual), but the repo consistently uses VERIFY_ARE_EQUAL(actual, expected) (e.g., VERIFY_ARE_EQUAL(GetLastError(), ERROR_SERVICE_ALREADY_RUNNING)). Update the macro descriptions (and any examples) to match the existing ordering to avoid confusing/asserting the wrong side in failure output.

Copilot uses AI. Check for mistakes.
- Validate copyright headers: `python3 tools/devops/validate-copyright-headers.py`
- **Note**: Will report missing headers in generated/dependency files (`_deps/`), which is expected
- Validate localization: `python3 tools/devops/validate-localization.py`
- **Note**: Only works after Windows build (requires `localization/strings/en-us/Resources.resw`)
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The localization validation note references localization/strings/en-us/Resources.resw, but the repo directory is localization/strings/en-US/Resources.resw (case-sensitive on non-Windows filesystems). Please fix the path casing so the instruction is correct.

Suggested change
- **Note**: Only works after Windows build (requires `localization/strings/en-us/Resources.resw`)
- **Note**: Only works after Windows build (requires `localization/strings/en-US/Resources.resw`)

Copilot uses AI. Check for mistakes.
Comment on lines +272 to +277
- **Note**: May show warnings about mermaid CDN access on restricted networks

### Manual Validation Requirements
- **Windows builds**: Install MSI and test basic WSL functionality (`wsl --version`, `wsl -l`)
- **Documentation changes**: Review generated HTML in `doc/site/`
- **Distribution changes**: Test with actual WSL distribution installation
### Code Formatting and Validation (Works on Linux/Windows)
- Format all source: `.\FormatSource.ps1`
- Validate copyright headers: `python3 tools/devops/validate-copyright-headers.py`
- **Note**: Will report missing headers in generated/dependency files (`_deps/`), which is expected
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

This section says code formatting "Works on Linux/Windows" and instructs running .\FormatSource.ps1, but that script is generated by the Windows CMake configure step and CMake requires Visual Studio + clang-format.exe. Consider either (a) scoping FormatSource.ps1 to Windows and noting it appears after cmake ., and/or (b) keeping a clang-format CLI option for Linux-only formatting/validation.

Copilot uses AI. Check for mistakes.
Comment thread .editorconfig
Comment on lines +6 to +12
[*]
indent_style = space
indent_size = 4
end_of_line = crlf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

end_of_line = crlf is applied globally, which conflicts with existing LF-terminated Linux shell scripts (e.g., test/linux/unit_tests/build_tests.sh) and can break execution on Unix-like systems if editors rewrite them to CRLF. Please add per-file overrides (at least *.sh/*.bash/*.py/*.cmake -> LF) or change the default to LF and opt-in CRLF only for Windows-specific file types (e.g., .resw, .idl, .ps1).

Copilot uses AI. Check for mistakes.
@benhillis benhillis marked this pull request as draft April 5, 2026 14:56
@benhillis benhillis marked this pull request as ready for review April 5, 2026 14:56
@benhillis benhillis closed this Apr 5, 2026
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