Update copilot-instructions.md with learnings from recent PRs#10974
Open
Update copilot-instructions.md with learnings from recent PRs#10974
Conversation
- 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>
rmarinho
requested review from
grendello,
jonathanpeppers and
simonrozsival
as code owners
Contributor
There was a problem hiding this comment.
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) vsAndroidTask. - Expand error-pattern guidance (resource-based messages, error code lifecycle, and
AsyncTasklogging 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.
Member
There was a problem hiding this comment.
Maybe remove the submodule bump here, the instructions changes look fine on their own.
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Updates
.github/copilot-instructions.mdwith learnings from recent PRs (#10949, #10969, #10971 and dotnet/android-tools#312).Changes
Architecture
external/xamarin-android-tools/submodule — it contains key shared infrastructure (AndroidTask/AsyncTaskbase classes,AdbRunner,EmulatorRunner, NRT extensions,CreateTaskLogger)MSBuild Tasks
AsyncTaskshould be used for tasks needingasync/await(not justAndroidTask)Error Patterns (expanded section)
Properties.Resources— previously not documented, led to hardcoded strings in AI-generated codeXA####code; don't leave orphaned entries inResources.resxLogCodedError()/LogMessage()helpers instead ofLog.*which is[Obsolete]onAsyncTaskand can hang Visual StudioNullable Reference Types
!workarounds for test code — document how to avoidnull!in[SetUp]fields (use nullable types) and afterAssert.IsNotNull(extract to local variable)Context
These gaps caused real issues during PR development:
Log.LogCodedErrorfromAsyncTaskasync context (should useLogCodedError)null!pattern in test fields flagged by all 4 AI models during multi-model code reviewProperties.Resources