Skip to content

Remove Pure attribute from System.Collections.Immutable - #35118

Merged
stephentoub merged 2 commits into
dotnet:masterfrom
ericstj:immutable-pure
Apr 27, 2020
Merged

Remove Pure attribute from System.Collections.Immutable#35118
stephentoub merged 2 commits into
dotnet:masterfrom
ericstj:immutable-pure

Conversation

@ericstj

@ericstj ericstj commented Apr 17, 2020

Copy link
Copy Markdown
Member

No description provided.

@ghost

ghost commented Apr 17, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @eiriktsarpalis
Notify danmosemsft if you want to be subscribed.

@ericstj

ericstj commented Apr 17, 2020

Copy link
Copy Markdown
Member Author

Fixes #28488

@ericstj ericstj added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Apr 17, 2020
@ericstj

ericstj commented Apr 17, 2020

Copy link
Copy Markdown
Member Author

Tagged as breaking since the issue mentions that the purpose of this is to raise CA1806 warnings.

Comment thread src/libraries/System.Collections.Immutable/Directory.Build.props Outdated
Comment thread src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.cs Outdated
@ericstj ericstj changed the title Preserve Pure attribute in System.Collections.Immutable Remove Pure attribute from System.Collections.Immutable Apr 20, 2020
@ericstj

ericstj commented Apr 20, 2020

Copy link
Copy Markdown
Member Author

So I changed the title of this PR to remove the usage of Pure. @AArnott / @stephentoub what do you think is the right way to track the scenario here?

@stephentoub stephentoub removed the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Apr 20, 2020
@stephentoub

stephentoub commented Apr 20, 2020

Copy link
Copy Markdown
Member

what do you think is the right way to track the scenario here?

First decide what the scenario is 😄 If it's "warn when methods whose results shouldn't be ignored are ignored", we should have an issue tracking/designing it (I think @terrajobst already opened one somewhere for that). If the scenario is something else, well, same answer :)

@ericstj

ericstj commented Apr 20, 2020

Copy link
Copy Markdown
Member Author

First decide what the scenario is 😄 If it's "warn when methods whose results shouldn't be ignored are ignored", we should have an issue tracking/designing it (I think @terrajobst already opened one somewhere for that). If the scenario is something else, well, same answer :)

Well I was expecting @AArnott to help fill that in once we have some direction about where to do so. I imagine the one you described is close enough to what I'm imagining @AArnott is looking for. @terrajobst can you link us up with that issue?

@terrajobst

Copy link
Copy Markdown
Contributor

I imagine the one you described is close enough to what I'm imagining @AArnott is looking for. @terrajobst can you link us up with that issue?

That would be #34098. Based on the experience of @stephentoub & @jaredpar in Midori I'd say the direction of having an attribute like [DoNotIgnoreReturnValue] seems like the right direction.

@stephentoub
stephentoub merged commit 90cb824 into dotnet:master Apr 27, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
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.

6 participants