Skip to content

Use String.Contains(char) instead of String.Contains(String)#36310

Closed
Youssef1313 wants to merge 2 commits intodotnet:masterfrom
Youssef1313:contains
Closed

Use String.Contains(char) instead of String.Contains(String)#36310
Youssef1313 wants to merge 2 commits intodotnet:masterfrom
Youssef1313:contains

Conversation

@Youssef1313
Copy link
Member

Similar to one of the changes in #36304

@stephentoub
Copy link
Member

stephentoub commented May 12, 2020

@Youssef1313, I appreciate your interest in contributing, but a bunch of these changes are to tests or tools where it really doesn't matter which overload is used, and others are to source (some src, some tests) that builds against versions of .NET that don't have the relevant overload, so it's going to fail to build.

@Youssef1313
Copy link
Member Author

@Youssef1313, I appreciate your interest in contributing, but a bunch of these changes are to tests or tools where it really doesn't matter which overload is used, and others are to product src that builds against versions of .NET that don't have the relevant overload, so it's going to fail to build.

If not all of the changes doesn't matter, I can update the PR to exclude only the files that belong to tests and where it doesn't matter.

If it doesn't matter in all of them, you can close the PR.

@stephentoub
Copy link
Member

If you'd like to remove the cases that fail to compile, we can see what's left. From a quick skim, I'm expecting most of the changes will end up getting reverted.

@stephentoub
Copy link
Member

@Youssef1313, given it's been a few weeks, I'm going to go ahead and close this. If you'd like to submit a PR that just handles such changes for product code under libraries, we can try to get that in. Thanks for your interest.

@stephentoub stephentoub closed this Jun 1, 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.

3 participants