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 UseStringMethodCharOverloadWithSingleCharacters analyzer and fixer #6799

Merged
merged 25 commits into from Jul 26, 2023

Conversation

mrahhal
Copy link
Contributor

@mrahhal mrahhal commented Jul 25, 2023

@mrahhal mrahhal requested a review from a team as a code owner July 25, 2023 16:06
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #6799 (f159d4c) into main (b3274a6) will increase coverage by 0.00%.
Report is 9 commits behind head on main.
The diff coverage is 95.27%.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6799    +/-   ##
========================================
  Coverage   96.34%   96.34%            
========================================
  Files        1386     1394     +8     
  Lines      326229   327150   +921     
  Branches    10729    10768    +39     
========================================
+ Hits       314296   315206   +910     
- Misses       9225     9230     +5     
- Partials     2708     2714     +6     

@tarekgh
Copy link
Member

tarekgh commented Jul 25, 2023

@mrahhal let's know when you finish all coding so we can review it in one shot. Thanks!

@mrahhal
Copy link
Contributor Author

mrahhal commented Jul 25, 2023

@tarekgh Resolved all the pending conversations (except for the severity thing). Ready for another round of review.

@tarekgh
Copy link
Member

tarekgh commented Jul 26, 2023

@Youssef1313 @buyaa-n it looks you are already reviewing the PR. Thanks for that. I finished reviewing and LGTM. Please let me know if you have any more comments here.

@stephentoub do you want to take a quick look?

CC @carlossanlop as you have added some analyzer and may have any comment.

@carlossanlop carlossanlop enabled auto-merge (squash) July 26, 2023 22:07
@carlossanlop carlossanlop merged commit b499986 into dotnet:main Jul 26, 2023
11 checks passed
@tarekgh
Copy link
Member

tarekgh commented Jul 26, 2023

Thanks @mrahhal for helping implementing it.

Are you interested in playing more with the analyzers? dotnet/runtime#78406 is a good one you may consider if you are interested and if @Youssef1313 not doing it :-)

@mrahhal
Copy link
Contributor Author

mrahhal commented Jul 27, 2023

Thanks for the reviews. Yes I'm planning to look at more from the table.

Working on the docs PR for this, will post here.

@mrahhal mrahhal deleted the use-string-method-char-overload branch July 27, 2023 06:06
@mrahhal
Copy link
Contributor Author

mrahhal commented Jul 27, 2023

Docs PR: dotnet/docs#36417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Analyzer] Recommend string.StartsWith(c) instead of string.StartsWith("c")
6 participants