Skip to content

Commit e6976c5

Browse files
Refine FieldWorks AI review workflows (#905)
* Add CONTEXT.md to integrate grill-with-docs into planning/OpenSpec flows so ambiguous repo language is clarified before proposal/spec/design/task artifacts are written * Add experimental AI PR workflow docs and prompts; introduce pr-preflight and respond-to-review-comments skills; update repo guidance and templates. --------- Co-authored-by: Jason Naylor <jason_naylor@sil.org>
1 parent 026ca3d commit e6976c5

24 files changed

Lines changed: 1122 additions & 44 deletions

File tree

.github/context/codebase.context.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
Use these entry points to load context efficiently without scanning the entire repo.
44

5-
- Onboarding: `.github/AGENTS.md`
5+
- Shared language and glossary: `CONTEXT.md`
6+
- Onboarding (canonical anchor): `.github/AGENTS.md`
67
- Src catalog (overview of major folders): `.github/src-catalog.md`
8+
- AI review workflow: `grill-with-docs` for planning language, `.github/instructions/review-analyzer.instructions.md` for review policy, `pr-preflight` for branch/PR readiness, and `respond-to-review-comments` for reviewer feedback.
9+
- Specialist review agents: `.github/agents/fieldworks.csharp-expert.agent.md`, `.github/agents/fieldworks.winforms-expert.agent.md`, `.github/agents/fieldworks.cpp-expert.agent.md`, and `.github/agents/fieldworks.avalonia-expert.agent.md`.
710
- Component guides: `Src/<Folder>/AGENTS.md` (and subfolder AGENTS.md where present)
811
- Build system: `build.ps1`, `FieldWorks.proj`, `FieldWorks.sln`, `Build/InstallerBuild.proj`
912
- Installer: `FLExInstaller/`
@@ -15,4 +18,4 @@ Use these entry points to load context efficiently without scanning the entire r
1518
Tips
1619
- Prefer top-level scripts or FieldWorks.sln over ad-hoc project builds
1720
- Respect CI checks (commit messages, whitespace) before pushing
18-
21+
- Prefer FieldWorks-specific agents over generic language/framework agents when reviewing this repo

.github/copilot-instructions.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ desktop application.
1616
validation, encoding assumptions, buffer lengths, and ownership rules.
1717
- Verify user-visible UI strings are placed in `.resx` resources and remain
1818
compatible with Crowdin localization.
19-
- For C# code, flag C# 8+ syntax and APIs that are not compatible with .NET
20-
Framework 4.8 / C# 7.3.
19+
- For C# code, flag language features newer than the repo default C# 8.0,
20+
nullable-reference-type assumptions when nullable is disabled, and APIs that
21+
are not compatible with .NET Framework 4.8.
2122
- For legacy `.csproj` changes, verify new source files are explicitly included
2223
and test sources are explicitly excluded where production and test trees mix.
2324
- Require meaningful test or validation evidence for bug fixes, regressions,

.github/instructions/dotnet-framework.instructions.md

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,50 +6,55 @@ applyTo: '**/*.csproj, **/*.cs'
66
# .NET Framework Development
77

88
## Build and Compilation Requirements
9-
- Always use `msbuild /t:rebuild` to build the solution or projects instead of `dotnet build`
9+
- Use `./build.ps1` for normal builds and `./test.ps1` for normal test runs.
10+
- Do not use `dotnet build` for the main repo workflow.
11+
- Avoid direct `msbuild` or project-only builds unless you are explicitly debugging build infrastructure.
1012

1113
## Project File Management
1214

13-
### Non-SDK Style Project Structure
14-
.NET Framework projects use the legacy project format, which differs significantly from modern SDK-style projects:
15+
### SDK-style net48 projects are common
16+
Most managed projects in this repo use SDK-style `.csproj` files while still targeting .NET Framework `net48`.
1517

16-
- **Explicit File Inclusion**: All new source files **MUST** be explicitly added to the project file (`.csproj`) using a `<Compile>` element
17-
- .NET Framework projects do not automatically include files in the directory like SDK-style projects
18-
- Example: `<Compile Include="Path\To\NewFile.cs" />`
18+
- **Implicit file inclusion is common**: SDK-style projects usually include new `.cs` files automatically.
19+
- **Check the touched project before editing**: Some projects still carry explicit includes, linked files, designer items, generated code, or custom metadata that must be preserved.
20+
- **Assembly metadata is managed explicitly**: Keep `GenerateAssemblyInfo` disabled where the project links `CommonAssemblyInfo.cs`.
21+
- **Preserve project-specific settings**: Keep existing settings for x64, WinExe, WindowsDesktop, COM, resources, warnings-as-errors, and custom MSBuild targets.
22+
- **Do not normalize projects unnecessarily**: Avoid converting project structure, import style, or target declarations unless that is the task.
1923

20-
- **No Implicit Imports**: Unlike SDK-style projects, .NET Framework projects do not automatically import common namespaces or assemblies
24+
### Legacy project caveat
25+
Some non-SDK-style or tool-specific projects may still exist. When you encounter one:
2126

22-
- **Build Configuration**: Contains explicit `<PropertyGroup>` sections for Debug/Release configurations
27+
- **No implicit imports**: Unlike SDK-style projects, legacy .NET Framework projects may not automatically import common namespaces or assemblies.
28+
- **Explicit build configuration**: Legacy projects often contain explicit `<PropertyGroup>` sections for Debug/Release configurations.
2329

24-
- **Output Paths**: Explicit `<OutputPath>` and `<IntermediateOutputPath>` definitions
25-
26-
- **Target Framework**: Uses `<TargetFrameworkVersion>` instead of `<TargetFramework>`
27-
- Example: `<TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>`
30+
- Keep explicit `<Compile Include=... />` items and other legacy structure intact.
31+
- Update the project file only as much as the change requires.
32+
- Do not assume you can migrate it to SDK-style as part of routine code edits.
2833

2934
## NuGet Package Management
3035
- Installing and updating NuGet packages in .NET Framework projects is a complex task requiring coordinated changes to multiple files. Therefore, **do not attempt to install or update NuGet packages** in this project.
3136
- Instead, if changes to NuGet references are required, ask the user to install or update NuGet packages using the Visual Studio NuGet Package Manager or Visual Studio package manager console.
3237
- When recommending NuGet packages, ensure they are compatible with .NET Framework or .NET Standard 2.0 (not only .NET Core or .NET 5+).
3338

34-
## C# Language Version is 7.3
35-
- This project is limited to C# 7.3 features only. Please avoid using:
39+
## C# Language Version
40+
- The repo-wide default language version for C# projects is 8.0 via `Directory.Build.props`.
41+
- Do not introduce features that require a newer language version unless the specific project or repo policy is updated first.
42+
- Prefer syntax already used in the touched area so edits remain consistent with surrounding code.
43+
44+
### C# 8.0 features available by default
45+
- Switch expressions and pattern matching enhancements.
46+
- Using declarations where disposal scope remains clear.
47+
- Null-coalescing assignment and range/index operators when they improve clarity.
3648

37-
### C# 8.0+ Features (NOT SUPPORTED):
38-
- Using declarations (`using var stream = ...`)
39-
- Await using statements (`await using var resource = ...`)
40-
- Switch expressions (`variable switch { ... }`)
41-
- Null-coalescing assignment (`??=`)
42-
- Range and index operators (`array[1..^1]`, `array[^1]`)
43-
- Default interface methods
44-
- Readonly members in structs
45-
- Static local functions
46-
- Nullable reference types (`string?`, `#nullable enable`)
49+
### Features not safe to assume repo-wide
50+
- Nullable reference types are not enabled repo-wide. Do not introduce `string?`, `#nullable enable`, or nullable-flow assumptions unless the project explicitly opts in.
51+
- File-scoped namespaces are not available under the repo default language version and should not be introduced.
4752

4853
### C# 9.0+ Features (NOT SUPPORTED):
4954
- Records (`public record Person(string Name)`)
5055
- Init-only properties (`{ get; init; }`)
5156
- Top-level programs (program without Main method)
52-
- Pattern matching enhancements
57+
- Pattern matching enhancements beyond C# 8
5358
- Target-typed new expressions (`List<string> list = new()`)
5459

5560
### C# 10+ Features (NOT SUPPORTED):
@@ -58,17 +63,15 @@ applyTo: '**/*.csproj, **/*.cs'
5863
- Record structs
5964
- Required members
6065

61-
### Use Instead (C# 7.3 Compatible):
62-
- Traditional using statements with braces
63-
- Switch statements instead of switch expressions
64-
- Explicit null checks instead of null-coalescing assignment
65-
- Array slicing with manual indexing
66-
- Abstract classes or interfaces instead of default interface methods
66+
### Use instead when staying within repo defaults
67+
- Block-scoped namespaces
68+
- Explicit null checks unless the project has enabled nullable analysis
69+
- Established project patterns over newer syntax for its own sake
6770

6871
## Environment Considerations (Windows environment)
69-
- Use Windows-style paths with backslashes (e.g., `C:\path\to\file.cs`)
70-
- Use Windows-appropriate commands when suggesting terminal operations
71-
- Consider Windows-specific behaviors when working with file system operations
72+
- FieldWorks builds and managed test runs are Windows-only.
73+
- On non-Windows hosts, limit work to editing, code search, docs, and specs.
74+
- When suggesting build or test commands, prefer the repo PowerShell scripts and Windows-oriented workflows.
7275

7376
## Common .NET Framework Pitfalls and Best Practices
7477

0 commit comments

Comments
 (0)