Skip to content

Seal overridden virtual method SetDefaultFallbacks #100030

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 1, 2024
Merged

Seal overridden virtual method SetDefaultFallbacks #100030

merged 1 commit into from
Jul 1, 2024

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Mar 20, 2024 •
edited
Loading

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2024
This was referenced Mar 21, 2024
Closed
Closed
Open
Closed
Closed
Closed
@build-analysis build-analysis bot mentioned this pull request Mar 30, 2024
Closed
This was referenced Apr 6, 2024
Closed
Closed
Closed
Closed
Closed
This was referenced Apr 19, 2024
Closed
Closed
Closed
Open
Closed
This was referenced May 1, 2024
Closed
Open
Closed
@xtqqczze
Copy link
Contributor Author

xtqqczze commented May 2, 2024

Build Analysis is green.

@stephentoub
Copy link
Member

Thanks, but what does this improve?

@stephentoub stephentoub added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 14, 2024
@xtqqczze
Copy link
Contributor Author

xtqqczze commented May 25, 2024 •
edited
Loading

Thanks, but what does this improve?

SetDefaultFallbacks is sealed in ASCIIEncoding, UTF7Encoding, UTF8Encoding, UnicodeEncoding. This change provides for consistency with these classes and clarifies that the method is not intended to be overridden.

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 25, 2024
@xtqqczze
Copy link
Contributor Author

@MihuBot

@MihuBot MihuBot mentioned this pull request May 25, 2024
Open
@xtqqczze
Copy link
Contributor Author

This change enables inlining of SetDefaultFallbacks into the ctor:

MihuBot/runtime-utils#386 (comment)

@xtqqczze xtqqczze force-pushed the SetDefaultFallbacks branch from 4752b20 to c3fbca4 Compare June 11, 2024 11:16
This was referenced Jun 11, 2024
Closed
Closed
Open
@stephentoub
Copy link
Member

Thanks, but what does this improve?

SetDefaultFallbacks is sealed in ASCIIEncoding, UTF7Encoding, UTF8Encoding, UnicodeEncoding. This change provides for consistency with these classes and clarifies that the method is not intended to be overridden.

That's fair. Thanks

@stephentoub stephentoub reopened this Jun 23, 2024
This was referenced Jun 23, 2024
Closed
Closed
Closed
@stephentoub stephentoub merged commit 9cc0e75 into dotnet:main Jul 1, 2024
@xtqqczze xtqqczze deleted the SetDefaultFallbacks branch July 2, 2024 10:47
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoub stephentoub stephentoub approved these changes

Assignees

No one assigned

Labels

area-System.Text.Encoding community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xtqqczze @stephentoub