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.

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

Replace some single-char strings with chars #81831

Merged
merged 3 commits into from
Feb 13, 2023
Merged

Replace some single-char strings with chars #81831

merged 3 commits into from
Feb 13, 2023

Conversation

stephentoub
Copy link
Member

Also a few other minor improvements noticed along the way.

@stephentoub
Replace some single-char strings with chars
3b2f224 
Also a few other minor improvements noticed along the way.
@ghost ghost assigned stephentoub Feb 8, 2023
@ghost
Copy link

ghost commented Feb 8, 2023

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

Also a few other minor improvements noticed along the way.

Author: stephentoub
Assignees: stephentoub
Labels:

area-Meta
Milestone: -

@runfoapp runfoapp bot mentioned this pull request Feb 8, 2023
Closed
@build-analysis build-analysis bot mentioned this pull request Feb 9, 2023
Closed
@stephentoub stephentoub mentioned this pull request Feb 9, 2023
Closed
@build-analysis build-analysis bot mentioned this pull request Feb 9, 2023
Closed
tannergooding
tannergooding approved these changes Feb 13, 2023
View reviewed changes
Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

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

Should we have an analyzer for any of this?

I believe I also saw you opened an issue on Roslyn for them to optimize something like this, is that right?

@stephentoub
Copy link
Member Author

I believe I also saw you opened an issue on Roslyn for them to optimize something like this, is that right?

Yup, I opened a Roslyn issue about having the compiler emit string.Concat(..., new ReadOnlySpan<char>(in c), ...). If that eventually happens, we can revert those changes here.

Should we have an analyzer for any of this?

#78392

@stephentoub stephentoub merged commit a491daa into dotnet:main Feb 13, 2023
@stephentoub stephentoub deleted the usesinglechar branch February 13, 2023 20:22
@build-analysis build-analysis bot mentioned this pull request Feb 13, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kronic kronic kronic left review comments

@EgorBo EgorBo EgorBo left review comments

@f1nzer f1nzer f1nzer left review comments

@lordfanger lordfanger lordfanger left review comments

@MichalPetryka MichalPetryka MichalPetryka left review comments

@tannergooding tannergooding tannergooding approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees

@stephentoub stephentoub

Labels
area-Meta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@stephentoub @kronic @EgorBo @f1nzer @tannergooding @lordfanger @MichalPetryka