Skip to content

Update comment for Async covariant return issue#126686

Merged
MichalStrehovsky merged 1 commit intomainfrom
MichalStrehovsky-patch-1
Apr 9, 2026
Merged

Update comment for Async covariant return issue#126686
MichalStrehovsky merged 1 commit intomainfrom
MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

No description provided.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

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 NativeAOT build-disable rationale in the async covariant return test project by replacing a generic “NYI” note with a direct link to the tracked runtime issue.

Changes:

  • Replaced the csproj comment with a link to dotnet/runtime issue #126685.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 9, 2026

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126686

Holistic Assessment

Motivation: Justified. The test was disabled with a vague "NYI" comment and now has a proper tracking issue (#126685 — "Covariant returns with runtime async," labeled disabled-test, area-NativeAOT-coreclr, runtime-async, milestoned 11.0.0).

Approach: Correct. Replacing an informal comment with a GitHub issue link is the standard convention for disabled tests in this repo (matches the disabled-test label pattern).

Summary: ✅ LGTM. This is a comment-only change in a test .csproj file with zero behavioral impact. The new comment links to a properly filed and labeled tracking issue. No other async test disables need the same treatment — the only other NativeAOT disable (capacity/capacity.csproj) already has a self-documenting reason (Reflection.Emit unsupported).


Detailed Findings

✅ Correctness — No behavioral change

The DisableProjectBuild condition is untouched. Only the XML comment above it changed. No risk of regression.

✅ Cross-cutting — Other disabled async tests checked

Searched all DisableProjectBuild entries under src/tests/async/. The only other NativeAOT disable (capacity/capacity.csproj) documents a fundamental platform limitation (Reflection.Emit) and doesn't need an issue link. No gaps found.

✅ Issue quality — Tracking issue is well-structured

Issue #126685 has appropriate labels (disabled-test, area-NativeAOT-coreclr, runtime-async), a milestone (11.0.0), and references the parent issue (#125900). The disabled-test label will allow automated tracking of this gap.

Generated by Code Review for issue #126686 ·

Copy link
Copy Markdown
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM. I believe the NativeAOT support for this is already in progress, so I did not log a bug.

@MichalStrehovsky
Copy link
Copy Markdown
Member Author

/ba-g timeouts but this is a comment only change

@MichalStrehovsky MichalStrehovsky merged commit 1653a0f into main Apr 9, 2026
84 of 94 checks passed
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch April 9, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants