Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2 of WinForms Async experimental APIs #11854

Merged
merged 9 commits into from
Aug 11, 2024

Conversation

KlausLoeffelmann
Copy link
Member

@KlausLoeffelmann KlausLoeffelmann commented Aug 10, 2024

Introduces new experimental APIs for .NET 9:

Control.InvokeAsync.
TaskDialog.ShowDialogAsync.
Form.ShowAsync.
Form.ShowDialogAsync

Fixes #10739.
Replaces: #11828
Replaces: #9827

@KlausLoeffelmann KlausLoeffelmann added this to the 9.0 RC1 milestone Aug 10, 2024
@KlausLoeffelmann KlausLoeffelmann self-assigned this Aug 10, 2024
@dotnet-policy-service dotnet-policy-service bot added the draft draft PR label Aug 10, 2024
Copy link

codecov bot commented Aug 10, 2024

Codecov Report

Attention: Patch coverage is 4.50450% with 212 lines in your changes missing coverage. Please review.

Project coverage is 74.81867%. Comparing base (d7c8152) to head (94de3b9).
Report is 3 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11854         +/-   ##
===================================================
- Coverage   74.84593%   74.81867%   -0.02726%     
===================================================
  Files           3015        3016          +1     
  Lines         629433      629649        +216     
  Branches       46717       46737         +20     
===================================================
- Hits          471105      471095         -10     
- Misses        154965      155185        +220     
- Partials        3363        3369          +6     
Flag Coverage Δ
Debug 74.81867% <4.50450%> (-0.02726%) ⬇️
integration 18.00400% <1.81818%> (-0.03904%) ⬇️
production 47.81081% <3.63636%> (-0.03888%) ⬇️
test 97.00749% <100.00000%> (-0.00114%) ⬇️
unit 44.83707% <3.63636%> (-0.03735%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@KlausLoeffelmann KlausLoeffelmann marked this pull request as ready for review August 11, 2024 01:42
@KlausLoeffelmann KlausLoeffelmann requested a review from a team as a code owner August 11, 2024 01:42
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

The locking in Form is the only thing I'm particularly concerned about.

@dotnet-policy-service dotnet-policy-service bot added 📭 waiting-author-feedback The team requires more information from the author and removed 📭 waiting-author-feedback The team requires more information from the author draft draft PR labels Aug 11, 2024
@@ -5,6 +5,8 @@ namespace System.Windows.Forms.Analyzers.Diagnostics;

internal static class DiagnosticIDs
{
public const string UrlFormat = "https://aka.ms/winforms-experimental/{0}";
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I missed this. We should make this more generic like Runtime does:

Suggested change
public const string UrlFormat = "https://aka.ms/winforms-experimental/{0}";
public const string UrlFormat = "https://aka.ms/winforms-warnings/{0}";

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking is that we later have more AKAs, like winforms-experimental and winforms-warnings and ... so we can have more landing pages for categories. But I have no preference. If you want this all warnings, I can create a new alias.

Copy link
Member

Choose a reason for hiding this comment

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

Let's start with following the libraries pattern.

Copy link
Member

Choose a reason for hiding this comment

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

NB: now you have two copies of the same const:
image

Copy link
Member

Choose a reason for hiding this comment

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

@RussKie thanks for pointing this out. We'll clean it up.

@KlausLoeffelmann KlausLoeffelmann force-pushed the WinFormsAsync2 branch 2 times, most recently from d70819b to 36e5c27 Compare August 11, 2024 20:26
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Looks good!

@KlausLoeffelmann KlausLoeffelmann merged commit 1ff332e into dotnet:main Aug 11, 2024
8 checks passed
@JeremyKuhne
Copy link
Member

Fixes #4631

@Olina-Zhang
Copy link
Member

Verified it in Winforms Release/9.0 branch, we can see these Async APIs.

@Zheng-Li01
Copy link
Member

Verified the issue with .NET 9.0.100-rc.1.24422.10 test pass build, for now the InvokeAsync. ShowAsync, ShowDialogAsync APIs has been implemented.

@Zheng-Li01
Copy link
Member

Verified the issue with .NET 9.0.100-rc.1.24452.12 test pass build, for now the InvokeAsync. ShowAsync, ShowDialogAsync APIs has been implemented.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Umbrella-Item] Planned "async feature" work for .NET 9
5 participants