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.

Sign up for GitHub

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

Jump to bottom

Ensure MacCatalyst default http handler is the same as iOS #52229

Merged
merged 5 commits into from
May 11, 2021
Merged

Ensure MacCatalyst default http handler is the same as iOS #52229

merged 5 commits into from
May 11, 2021

Conversation

steveisok
Copy link
Member

No description provided.

@ghost
Copy link

ghost commented May 4, 2021

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

Issue Details
Author: steveisok
Assignees: -
Labels:

area-System.Net.Http
Milestone: -

@steveisok steveisok requested a review from akoeplinger May 4, 2021 00:40
@steveisok
Copy link
Member Author

steveisok commented May 4, 2021 •
edited
Loading

For MacCatalyst, the Xamarin.iOS type forwards into Xamarin.MacCatalyst, so I believe this will work correctly.

/cc @rolfbjarne

@rolfbjarne
Copy link
Member

For MacCatalyst, the Xamarin.iOS type forwards into Xamarin.MacCatalyst, so I believe this will work correctly.

/cc @rolfbjarne

The Xamarin.iOS assembly shipped for Xamarin.MacCatalyst is a compatibility assembly so that existing assemblies built for Xamarin.iOS work as well as possible. New code should target Xamarin.MacCatalyst, and not rely on this compatibility behavior.

filipnavara
filipnavara reviewed May 4, 2021
View reviewed changes
...libraries/System.Net.Http/src/System/Net/Http/HttpClient.CreateDefaultHandler.MacCatalyst.cs Outdated

if (handlerMethod == null)
{
Type? runtimeOptions = Type.GetType("ObjCRuntime.RuntimeOptions, Xamarin.iOS");
Copy link
Member

Choose a reason for hiding this comment

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

The assembly name needs to be updated

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm losing my mind... I updated the wrong file ;-)

filipnavara
filipnavara approved these changes May 4, 2021
View reviewed changes
Copy link
Member

@filipnavara filipnavara left a comment

Choose a reason for hiding this comment

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

FWIW LGTM

@akoeplinger akoeplinger merged commit 90f2cef into dotnet:main May 11, 2021
@steveisok steveisok deleted the maccat-optin-nativehttphandler branch May 11, 2021 18:39
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@akoeplinger akoeplinger akoeplinger approved these changes

@filipnavara filipnavara filipnavara approved these changes

Assignees
No one assigned
Labels
area-System.Net.Http
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@steveisok @rolfbjarne @akoeplinger @filipnavara @karelz