-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Code modernization in System.Linq #110910
base: main
Are you sure you want to change the base?
Conversation
This might be too big to review. How about merging the most impactful change first, i.e. IDE0161: Convert to file-scoped namespace. |
The repo doesn't use file-scoped namespaces; I don't think we want to change the style per library. |
src/libraries/System.Linq/src/System/Linq/AppendPrepend.SpeedOpt.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Linq/src/System/Linq/OrderedEnumerable.SpeedOpt.cs
Outdated
Show resolved
Hide resolved
@stephentoub do we want to consider switching the repo to file-scoped namespaces in the future? Would be nice if we could save ourselves one tab of indentation. |
@xtqqczze note that each change is a distinct commit, that should help. |
afair some *.cs files have multiple namespace scopes in the same file? |
It also makes servicing much harder. |
Thanks for the reviewing. Have removed the file-scoped namespaces, and corrected the other stuff. |
Yeah, FWIW in EF and Npgsql we try to do this kind of thing (and general large code cleanups) before we lock down a major release, to make servicing easier. It's true file-scoped namespaces do make git blaming slightly harder - I personally do think it's worth prioritizing code niceness with them, and worth the one-time pain (but I get that it's non-trivial and removed that commit). |
Can you elaborate? |
Just that before a release is locked down, we usually do an automated code cleanup, to align code to the repo settings in .editorconfig and similar; this fixes any violations that have accumulated over the year from contributions where some rule wasn't respected. The timing - just before locking down the major version - is specifically done to make servicing easier, compared to e.g. if we did the cleanup right after the cleanup, at which point any backport would risk a conflict because of styling etc. |
Here are three relatively impactful code cleanups, done automatically and split into three separate commits for easier reviewing/undoing etc.