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

Add RemoveOrNone to Dictionary extensions #822

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

FreeApophis
Copy link
Member

@FreeApophis FreeApophis commented Jan 29, 2025

The Remove is an Extension on the IDictionary interface and has an out-parameter returning the removed Item.

Even though it has a side effect it maps perfectly to the Try* Pattern.

@FreeApophis FreeApophis force-pushed the add-remove-or-none-on-dictionary branch from eb33b01 to 728a247 Compare January 29, 2025 09:11
@FreeApophis
Copy link
Member Author

I do not understand why PublicAPI check fails. The Signature was autogenerated and it looks the same as for GetValueOrNone.

@bash
Copy link
Member

bash commented Jan 29, 2025

The failing check is the package validation check from the .NET SDK, not the PublicApi analyzer :)

You can run dotnet build /p:ApiCompatGenerateSuppressionFile=true to update the suppressions, I think we've added notnull constraints before in cases where the BCL does so.

Reason: The problem is inheritet from the .NET SDK.
@FreeApophis
Copy link
Member Author

FreeApophis commented Jan 30, 2025

The dotnet build with the paramter alone did not work, but I got it to update the suppression file (it was either that i tried pack, or the addition of the value to the csproj file).

We already had 3 suppressions, two of them are also because of the SDK change in Dictionary.

@FreeApophis FreeApophis enabled auto-merge January 30, 2025 08:14
@FreeApophis FreeApophis merged commit e222552 into main Jan 30, 2025
9 checks passed
@FreeApophis FreeApophis deleted the add-remove-or-none-on-dictionary branch January 30, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants