-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Initial commit of System.Net.WebSockets.Client. #2511
Conversation
Thanks, Pat. I'm going to go through an add some comments; since this is a porting effort, I'll just note in comments if I think they're important enough to address prior to merging, or whether they're just things to potentially follow-up on later. For the former, up to you whether you address now or leave them for later. |
} | ||
} | ||
|
||
internal class SafeWinHttpHandle : SafeHandleZeroOrMinusOneIsInvalid | ||
internal class SafeWinHttpHandleWithCallback : SafeWinHttpHandle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sealed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LGTM |
@dotnet-bot test this please |
10c6957
to
53f53f5
Compare
@pgavlin Looks like there is an error in compiling your latest commit. |
It's an unrelated test failure on linux: #2539 |
53f53f5
to
2f2fae2
Compare
@dotnet-bot test this please |
The last failure was a package restore problem; S.N.WebSockets.Client built and passed testing. That said, I've giving the lab one more run at it. |
@dotnet-bot test this please |
This job failed due to #2539 as well, which has since been resolved. I'm going to go ahead and merge. |
Initial commit of System.Net.WebSockets.Client.
I realize there have been a bunch of unfortunate, unrelated issues happening, but I'd prefer if we stuck to getting a green CI before merging. If we stray from that there's risk of missing a second failure buried in the logs. |
Yes, this needs to continue to be our policy. Thanks for understanding. |
Initial commit of System.Net.WebSockets.Client. Commit migrated from dotnet/corefx@63da6ec
No description provided.