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

[QUIC] System.Net.Quic API made public #72031

Merged
merged 2 commits into from
Jul 13, 2022

Conversation

ManickaP
Copy link
Member

@ManickaP ManickaP commented Jul 12, 2022

System.Net.Quic removed from ASP transport package and made part of SDK ref.

@ViktorHofer is this enough to revert hiding System.Net.Quic ref and to remove it from ASP.NET transport package? Or am I missing something.

Also cc @JamesNK there probably will be follow up on your side. I assume only some clean up in project files.

cc: @CarnaViire @wfurt @rzikm

@ghost
Copy link

ghost commented Jul 12, 2022

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

Issue Details

System.Net.Quic removed from ASP transport package and made part of SDK ref.

@ViktorHofer is this enough to revert hiding System.Net.Quic ref and to remove it from ASP.NET transport package? Or am I missing something.

Also cc @JamesNK there probably will be follow up on your side. I assume only some clean up in project files.

Author: ManickaP
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@ViktorHofer
Copy link
Member

Yes that looks complete. Nit, please also remove the ProjectReference here:

<ProjectReference Include="$(LibrariesProjectRoot)System.Net.Quic\src\System.Net.Quic.csproj" />
(as it's now a default reference via the shared framework).

@@ -1,9 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<!-- Even though this library's contract isn't exposed in the shared framework, it is built as part of the shared framework build
(because the System.Net.Quic\src references this project) and hence need to define its dependencies. -->
<DisableImplicitFrameworkReferences>true</DisableImplicitFrameworkReferences>
</PropertyGroup>

<ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need to remove the RequiresPreviewFeaturesAttribute when exposing the assembly in the shared framework?

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 not sure I fully understand, but we want to have S.N.Quic as PreviewFeature, since we want to be able to tweak the API for .NET 8. I hope the way I did it is correct.

@ManickaP ManickaP marked this pull request as ready for review July 12, 2022 20:37
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@ManickaP ManickaP merged commit 4827ee8 into dotnet:main Jul 13, 2022
@ManickaP ManickaP deleted the mapichov/quic-public-api branch July 13, 2022 18:02
ManickaP added a commit to ManickaP/runtime that referenced this pull request Jul 13, 2022
* System.Net.Quic removed from ASP transport package and made part of SDK ref

* Removed manual references to System.Net.Quic.csproj
@ManickaP
Copy link
Member Author

@JamesNK and this is the last change which publishes S.N.Quic as part of shared framework (i.e. ref dll) and removes it from ASP transport package. Some project level cleanup will be needed.

carlossanlop pushed a commit that referenced this pull request Jul 13, 2022
…72031) (#72106)

* [QUIC] API QuicConnection (#71783)

* Listener comment; PreviewFeature attribute

* Feedback

* QuicConnection new API including compilable implementation

* Fixed logging

* Fixed S.N.Quic and S.N.Http tests

* Options now correspond to the issue

* Feedback

* Comments, PreviewFeature attribute and RemoteCertificate disposal.

* Preview feature attribute is assembly wide

* Some typos

* Fixed test with certificate

* Default values as constants

* Event handlers split into methods called via switch expression.

* Some more comments

* Unified unsafe usage

* Fixed some more tests

* Cleaned up some exceptions and resource strings.

* Feedback

* Latest greatest API proposal.

* Fixed Http solution

* Feedback

* [QUIC] API QuicStream (#71969)

* Quic stream API surface

* Fixed test compilation

* Fixed http test compilation

* HttpLoopbackConnection Dispose -> DisposeAsync

* QuicStream implementation

* Fixed some tests

* Fixed all QUIC and HTTP tests

* Fixed exception type for stream closed by connection close

* Feedback

* Fixed WebSocket.Client test build

* Feedback, test fixes

* Fixed build on framework and windows

* Fixed winhandler test

* Swap variable based on order in defining class

* Post merge fixes

* Feedback and build

* Reverted connection state to pass around abort error code

* Fixed exception type.

* [QUIC] System.Net.Quic API made public (#72031)

* System.Net.Quic removed from ASP transport package and made part of SDK ref

* Removed manual references to System.Net.Quic.csproj
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants