Skip to content

Nullable annotation for System.Windows.Extensions - #57896

Merged
stephentoub merged 12 commits into
dotnet:mainfrom
huoyaoyuan:system-windows-extensions
Nov 24, 2021
Merged

Nullable annotation for System.Windows.Extensions#57896
stephentoub merged 12 commits into
dotnet:mainfrom
huoyaoyuan:system-windows-extensions

Conversation

@huoyaoyuan

@huoyaoyuanhuoyaoyuan commented Aug 22, 2021

Copy link
Copy Markdown
Member

Part of #41720

@ghostghost added the community-contribution Indicates that the PR has been added by a community member label Aug 22, 2021
@ghost

ghost commented Aug 22, 2021

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost

ghost commented Aug 22, 2021

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@huoyaoyuan

huoyaoyuan commented Aug 25, 2021

Copy link
Copy Markdown
MemberAuthor

How to verify the ref source? I tried /t:GenerateReferenceAssemblySource but it has no effect.

@ViktorHofer

ViktorHofer commented Sep 7, 2021•
edited
Loading

Copy link
Copy Markdown
Member

cc @Anipik@ericstj (for some reason the bot didn't send out a notification).

stephentoub
stephentoub requested changes Sep 7, 2021
View reviewed changes
Comment threadsrc/libraries/System.Windows.Extensions/ref/System.Windows.Extensions.cs Outdated
Comment threadsrc/libraries/System.Windows.Extensions/ref/System.Windows.Extensions.cs Outdated
Comment threadsrc/libraries/System.Windows.Extensions/src/System/Media/SoundPlayer.cs Outdated
Comment threadsrc/libraries/System.Windows.Extensions/src/System/Media/SoundPlayer.cs
Comment threadsrc/libraries/System.Windows.Extensions/ref/System.Windows.Extensions.cs Outdated
Comment threadsrc/libraries/System.Windows.Extensions/src/System/Media/SoundPlayer.cs Outdated
Comment threadsrc/libraries/System.Windows.Extensions/src/System/Media/SoundPlayer.cs

if (_stream.CanSeek)
// DO NOT assert - NRE is expected for null stream
// See SoundPlayerTests.Load_NullStream_ThrowsNullReferenceException

@stephentoubstephentoubSep 7, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It'd be worth opening an issue for this to decide separately whether we should throw a better exception in that case.

Comment threadsrc/libraries/System.Windows.Extensions/ref/System.Windows.Extensions.cs
@huoyaoyuanhuoyaoyuan mentioned this pull request Sep 7, 2021
Open
@runfoapprunfoappBot mentioned this pull request Sep 24, 2021
Closed
@huoyaoyuan

huoyaoyuan commented Sep 27, 2021

Copy link
Copy Markdown
MemberAuthor

@stephentoub Is this now ready?

stephentoub
stephentoub approved these changes Oct 6, 2021
View reviewed changes

@stephentoubstephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@stephentoubstephentoub reopened this Oct 6, 2021
@runfoapprunfoappBot mentioned this pull request Oct 6, 2021
Closed
@ericstj

ericstj commented Oct 25, 2021

Copy link
Copy Markdown
Member

/azp run runtime

@azure-pipelines

azure-pipelinesBot commented Oct 25, 2021

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@huoyaoyuan

huoyaoyuan commented Nov 24, 2021

Copy link
Copy Markdown
MemberAuthor

@stephentoub ping on this

@stephentoub
stephentoub merged commit 89e3040 into dotnet:mainNov 24, 2021
@huoyaoyuan
huoyaoyuan deleted the system-windows-extensions branch November 25, 2021 07:41
This was referenced Nov 29, 2021
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@danmoseleydanmoseley mentioned this pull request Dec 3, 2021
Closed
@ghostghost locked as resolved and limited conversation to collaborators Dec 25, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

@stephentoubstephentoub

Labels

area-Microsoft.Win32community-contributionIndicates that the PR has been added by a community membernew-api-needs-documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@huoyaoyuan@ViktorHofer@ericstj@stephentoub