Add GitHub Copilot instructions and agent configurations#285
Add GitHub Copilot instructions and agent configurations#285
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds GitHub Copilot configuration files to provide AI assistance with repository-specific conventions and patterns. The files aim to guide Copilot on C# 9.0 language constraints, project structure, testing practices, and development patterns for the Xamarin.Android.Tools library.
Changes:
- Adds
.github/copilot-instructions.mdwith C# development guidelines - Adds
.github/CSharpExpert.agent.mdwith C#/.NET expert agent configuration - Adds
.github/principal-software-engineer.agent.mdwith principal-level engineering guidance
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
.github/copilot-instructions.md |
Project-specific C# guidelines including language version, nullable types, project structure, and API patterns |
.github/CSharpExpert.agent.md |
C#/.NET expert agent configuration with async patterns, testing, and code design rules |
.github/principal-software-engineer.agent.md |
Principal-level engineering guidance on design patterns, technical leadership, and debt management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
20076ed to
8872d18
Compare
- Remove applyTo header (should apply to all files, not just *.cs) - Update C# version guidance to respect LangVersion from .csproj - Clarify Arcade SDK status (legacy pinned version, not fully onboarded) - Remove ASP.NET-specific guidance (caching, pagination, auth testing) - Clean up testing section (remove API endpoint references) - Fix performance section to be library-appropriate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,26 +1,26 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
I would undo any changes to this file. Seems unrelated random changes.
There was a problem hiding this comment.
If you want to change to net10.0 that should be a new PR.
- Revert Directory.Build.props and azure-pipelines.yaml (net10 is separate PR) - Change LangVersion from 9.0 to latest in csproj - Remove C# 9.0 features section (no longer relevant with latest) - Remove Naming Conventions section (editorconfig handles it) - Always use file-scoped namespaces - Use System.Text.Json with source generators - Add nullable extension guidance for netstandard2.0 - Fix CSharpExpert.agent.md: NUnit not xUnit, dynamic TFM reference - Remove applyTo header, Arcade SDK references - Clean up ASP.NET-specific guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Revert Directory.Build.props and azure-pipelines.yaml (net10 is separate PR) - Change LangVersion from 9.0 to latest in csproj - Remove C# 9.0 features section (no longer relevant with latest) - Remove Naming Conventions section (editorconfig handles it) - Always use file-scoped namespaces - Use System.Text.Json with source generators - Add nullable extension guidance for netstandard2.0 - Fix CSharpExpert.agent.md: NUnit not xUnit, dynamic TFM reference - Remove applyTo header, Arcade SDK references - Clean up ASP.NET-specific guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Revert Directory.Build.props and azure-pipelines.yaml (net10 is separate PR) - Change LangVersion from 9.0 to latest in csproj - Remove C# 9.0 features section (no longer relevant with latest) - Remove Naming Conventions section (editorconfig handles it) - Always use file-scoped namespaces - Use System.Text.Json with source generators - Add nullable extension guidance for netstandard2.0 - Fix CSharpExpert.agent.md: NUnit not xUnit, dynamic TFM reference - Remove applyTo header, Arcade SDK references - Clean up ASP.NET-specific guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Update README: modernize for current project state - Update description with current APIs (SdkManager, runners, etc.) - Fix build badge URL to dnceng-public - Update repo link to dotnet/android - Fix .NET version requirement (6 -> 9) - Add multi-targeting section explaining netstandard2.0 requirement - Remove outdated Xamarin feed, mailing list, Mono coding guidelines - Simplify test command Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Update to .NET 10 - Directory.Build.props: DotNetTargetFrameworkVersion 9.0 -> 10.0 - azure-pipelines.yaml: DotNetCoreVersion 9.0.x -> 10.0.x - README.md and copilot-instructions.md: reference net10.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Revert "Update to .NET 10" This reverts commit 979699c.
5e40e04 to
6a4142b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Project-Specific Context | ||
|
|
||
| This is the **Xamarin.Android.Tools** library - tools for interacting with the Android SDK: | ||
| - Multi-targets: `netstandard2.0` and `$(DotNetTargetFramework)` (currently `net9.0`) |
There was a problem hiding this comment.
Typo: "netstandard2.0" is misspelled as "\netstandard2.0" (with backslashes instead of backticks). This should be netstandard2.0 for proper markdown formatting.
|
|
||
| - Apply code-formatting style defined in `.editorconfig` | ||
| - Use **tabs** for indentation in C# files (as per `.editorconfig`) | ||
| - Always use **file-scoped namespaces** (`namespace Xamarin.Android.Tools;`) |
There was a problem hiding this comment.
The instruction to "Always use file-scoped namespaces" conflicts with the existing codebase convention. All C# files in the repository use traditional block-scoped namespaces (namespace X { }), not file-scoped namespaces (namespace X;). The instructions should be updated to use block-scoped namespaces to match the existing convention found throughout the codebase.
| This is the **Xamarin.Android.Tools** library - tools for interacting with the Android SDK: | ||
| - Multi-targets: `netstandard2.0` and `$(DotNetTargetFramework)` (currently `net9.0`) | ||
| - **`netstandard2.0` is required** — this package is consumed by .NET Framework hosts (e.g., Visual Studio on Windows). All public API and core logic **must** compile against netstandard2.0. Use `#if NET5_0_OR_GREATER` guards for newer runtime features (e.g., `ArrayPool`, `Process.Kill(true)`) | ||
| - C# Language Version: `LangVersion=latest` — use the latest C# features available for each target framework |
There was a problem hiding this comment.
The statement "LangVersion=latest — use the latest C# features available for each target framework" may be misleading. While the csproj now uses LangVersion=latest, this doesn't mean developers should freely use all latest C# features. The project multi-targets netstandard2.0, which has limitations on what C# features can be used (e.g., file-scoped namespaces require C# 10+, which isn't fully supported in netstandard2.0 scenarios). Consider clarifying that "latest" refers to the latest language features compatible with each target framework's constraints.
- Fix LangVersion guidance: clarify netstandard2.0 constraints - Change file-scoped to block-scoped namespaces (matches existing codebase) - Remove System.Text.Json guidance (not used in project) - Fix nullable string handling: use null-coalescing operator instead of non-existent OrEmpty() extension Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add documentation to guide GitHub Copilot when working with this codebase: - .github/copilot-instructions.md: Project-specific coding guidelines - netstandard2.0 compatibility requirements - Block-scoped namespace convention (matches existing codebase) - Nullable reference types patterns - NUnit testing framework usage - Build and test commands - .github/CSharpExpert.agent.md: C# expert agent for code assistance - Code design rules and workflow guidance - Async best practices - Testing guidance with NUnit - README.md: Updated project description and build requirements Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1130086 to
bd335eb
Compare
|
Closing in favor of: If there is still something useful here, maybe we can put in a new PR -- or rebase/reopen. |
These files were part of a separate copilot instructions PR (#285/#287). Main now has its own copilot-instructions.md. Removing: - CSharpExpert.agent.md (not in main) - principal-software-engineer.agent.md (not in main) - copilot-instructions.md changes (reverted to main) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds GitHub Copilot instructions, agent configuration, and modernizes the README.
Changes
.github/copilot-instructions.md (new)
Project-specific guidelines for AI-assisted development:
netstandard2.0(required for .NET Framework / Visual Studio hosts) +net9.0LangVersion=latest, file-scoped namespacesSystem.Text.Jsonwith source generatorsINTERNAL_NULLABLE_ATTRIBUTESpolyfill for netstandard2.0.editorconfigrules, XML doc comments for public APIsAction<TraceLevel, string>delegates.github/CSharpExpert.agent.md (new)
Expert C#/.NET agent for this Android SDK tooling library:
netstandard2.0+$(DotNetTargetFramework))ConfigureAwait(false)README.md (updated)
xamarin/xamarin-androidtodotnet/android)Xamarin.Android.Tools.AndroidSdk.csproj
LangVersionfrom9.0tolatestTesting
No behavioral code changes - documentation and build configuration only. Build verified locally.