Skip to content

Update copilot-instructions.md with learnings from recent PRs#10974

Merged
jonathanpeppers merged 3 commits intomainfrom
dev/rumar/update-copilot-instructions
Mar 20, 2026
Merged

Update copilot-instructions.md with learnings from recent PRs#10974
jonathanpeppers merged 3 commits intomainfrom
dev/rumar/update-copilot-instructions

Conversation

@rmarinho
Copy link
Member

Summary

Updates .github/copilot-instructions.md with learnings from recent PRs (#10949, #10969, #10971 and dotnet/android-tools#312).

Changes

Architecture

  • Document external/xamarin-android-tools/ submodule — it contains key shared infrastructure (AndroidTask/AsyncTask base classes, AdbRunner, EmulatorRunner, NRT extensions, CreateTaskLogger)

MSBuild Tasks

  • Clarify that AsyncTask should be used for tasks needing async/await (not just AndroidTask)

Error Patterns (expanded section)

  • Error messages must come from Properties.Resources — previously not documented, led to hardcoded strings in AI-generated code
  • Error code lifecycle — when removing functionality, clean up or repurpose the XA#### code; don't leave orphaned entries in Resources.resx
  • AsyncTask logging — use thread-safe LogCodedError()/LogMessage() helpers instead of Log.* which is [Obsolete] on AsyncTask and can hang Visual Studio

Nullable Reference Types

  • ! workarounds for test code — document how to avoid null! in [SetUp] fields (use nullable types) and after Assert.IsNotNull (extract to local variable)

Context

These gaps caused real issues during PR development:

  • Using Log.LogCodedError from AsyncTask async context (should use LogCodedError)
  • Orphaned XA0144 error code after refactoring
  • null! pattern in test fields flagged by all 4 AI models during multi-model code review
  • Hardcoded error strings instead of Properties.Resources

Copilot AI review requested due to automatic review settings March 19, 2026 17:34
Copy link
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 .github/copilot-instructions.md to capture recent learnings about task base classes, error patterns, and nullable reference type guidance, and bumps the external/xamarin-android-tools submodule to a newer commit.

Changes:

  • Document external/xamarin-android-tools/ as shared SDK tooling and infrastructure.
  • Clarify guidance for when to use AsyncTask (async/await) vs AndroidTask.
  • Expand error-pattern guidance (resource-based messages, error code lifecycle, and AsyncTask logging best practices) and add NRT guidance for tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
external/xamarin-android-tools Updates the submodule pointer to a newer upstream commit.
.github/copilot-instructions.md Adds/clarifies guidance on repo architecture, MSBuild task patterns, error handling, and NRT usage in tests.

You can also share your feedback on Copilot code review. Take the survey.

rmarinho and others added 2 commits March 20, 2026 09:11
- Document external/xamarin-android-tools submodule in Architecture
- Add AsyncTask guidance (base class, thread-safe logging)
- Add error message localization requirement (Properties.Resources)
- Add error code lifecycle guidance (no orphaned XA codes)
- Add null-forgiving operator workarounds for test code
- Clarify AsyncTask vs AndroidTask base class usage

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Outdated emulator tools (pre-35.x) render a black screen on Apple
Silicon Macs. Document the fix: update via sdkmanager.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho rmarinho force-pushed the dev/rumar/update-copilot-instructions branch from 22f07e8 to 933bd43 Compare March 20, 2026 09:11
Copy link
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 repo’s AI contribution guidance (.github/copilot-instructions.md) based on recent MSBuild/emulator work, with emphasis on shared tooling, correct MSBuild task base classes, localized error patterns, and nullable-reference-type practices in tests.

Changes:

  • Document external/xamarin-android-tools/ as shared infrastructure used by this repo’s build/task ecosystem.
  • Clarify MSBuild task guidance to use AsyncTask when async/await is required.
  • Expand error/nullable/test guidance (localized Properties.Resources messages, XA code lifecycle, avoiding ! in tests) and add a macOS emulator troubleshooting note.

- **MSBuild Errors:** `XA####` (errors), `XA####` (warnings), `APT####` (Android tools)
- **Logging:** Use `Log.LogError`, `Log.LogWarning` with error codes and context
- **Error messages:** Must come from `Properties.Resources` (e.g., `Properties.Resources.XA0143`) for localization support. Add new messages to the English `Resources.resx` file.
- **Error code lifecycle:** When removing functionality that used an `XA####` code, either repurpose the code or remove it from `Resources.resx` and `Resources.Designer.cs`. Don't leave orphaned codes.
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The guidance to remove an XA#### entry from Resources.Designer.cs is misleading: this file is auto-generated and manual edits will be lost. Suggest updating this bullet to say to remove/repurpose the entry in Resources.resx and then regenerate the designer (rebuild/resgen), rather than editing Resources.Designer.cs directly.

Suggested change
- **Error code lifecycle:** When removing functionality that used an `XA####` code, either repurpose the code or remove it from `Resources.resx` and `Resources.Designer.cs`. Don't leave orphaned codes.
- **Error code lifecycle:** When removing functionality that used an `XA####` code, either repurpose the code or remove/rename the corresponding entry in `Resources.resx` and then regenerate `Resources.Designer.cs` (e.g., by rebuilding or running `resgen`). `Resources.Designer.cs` is auto-generated and should not be edited manually. Don't leave orphaned codes.

Copilot uses AI. Check for mistakes.
@jonathanpeppers jonathanpeppers merged commit cb014fa into main Mar 20, 2026
1 of 2 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/rumar/update-copilot-instructions branch March 20, 2026 13:31
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.

3 participants