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

Fix IDE0066 across corefx #40288

Merged
merged 2 commits into from Aug 16, 2019

Conversation

@stephentoub
Copy link
Member

commented Aug 13, 2019

I rather like the readability and brevity the new switch expression affords, so I auto-fixed it across all of corefx (I then reviewed each change and tweaked things in a few places where the rule got it "wrong"). There will likely still be more opportunity for further related refactorings, as the rule only applied to the one main project configuration I was working on, and the rule is also limited in what patterns it recognizes.

@bartonjs

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

I dunno, I find this rather... unpleasant.

@stephentoub

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

I dunno, I find this rather... unpleasant.

Really? What specifically? I like that it conveys the same information in less space, with conventions found elsewhere already in C#, avoiding unnecessary duplication (like assigning to the same variable on every branch or returning on every branch). I find it hard to find things not to like about it :)

@bartonjs

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

The biggest factors are probably that I'm (apparently) a luddite and/or that it's increasing the deviations from referencesource.microsoft.com (netfx) and source.dot.net (corefx).

I commented inline on a trailing comment stylistic thing; and one with long line lengths.

I guess if I tell myself it's like a dictionary initializer and remind myself that the arrow operator isn't actually creating an action and invoking it (which is the "convention found elsewhere" that appears in my head with this syntax) then it's "just different" and removes the need for the break keyword.

But I'm the King of Whitespace. I don't always thing "compact" means "clearer" :)

@bartonjs

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

I don't always thing "compact" means "clearer" :)

Spelling is, clearly, optional.

@stephentoub stephentoub force-pushed the stephentoub:ide0066 branch 3 times, most recently from ad267cf to f20d672 Aug 14, 2019

stephentoub added some commits Aug 13, 2019

Fix IDE0066 across corefx
"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

@stephentoub stephentoub force-pushed the stephentoub:ide0066 branch from f20d672 to e9195e8 Aug 16, 2019

@stephentoub stephentoub merged commit f71a5a1 into dotnet:master Aug 16, 2019

15 checks passed

WIP Ready for review
Details
corefx-ci Build #20190816.4 succeeded
Details
corefx-ci (Linux Build RedHat6_x64_Release) Linux Build RedHat6_x64_Release succeeded
Details
corefx-ci (Linux Build arm64_Debug) Linux Build arm64_Debug succeeded
Details
corefx-ci (Linux Build arm_Debug) Linux Build arm_Debug succeeded
Details
corefx-ci (Linux Build musl_arm64_Debug) Linux Build musl_arm64_Debug succeeded
Details
corefx-ci (Linux Build musl_x64_Debug) Linux Build musl_x64_Debug succeeded
Details
corefx-ci (Linux Build x64_Debug) Linux Build x64_Debug succeeded
Details
corefx-ci (Windows Build NETFX_x86_Release) Windows Build NETFX_x86_Release succeeded
Details
corefx-ci (Windows Build UWP_CoreCLR_x64_Debug) Windows Build UWP_CoreCLR_x64_Debug succeeded
Details
corefx-ci (Windows Build x64_Debug) Windows Build x64_Debug succeeded
Details
corefx-ci (Windows Build x86_Release) Windows Build x86_Release succeeded
Details
corefx-ci (Windows Packaging All Configurations x64_Debug) Windows Packaging All Configurations x64_Debug succeeded
Details
corefx-ci (macOS Build x64_Debug) macOS Build x64_Debug succeeded
Details
license/cla All CLA requirements met.
Details

@stephentoub stephentoub deleted the stephentoub:ide0066 branch Aug 16, 2019

Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/coreclr that referenced this pull request Aug 16, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Aug 16, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Aug 16, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

jkotas added a commit to dotnet/corert that referenced this pull request Aug 16, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

marek-safar added a commit to mono/mono that referenced this pull request Aug 17, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

jkotas added a commit to dotnet/coreclr that referenced this pull request Aug 18, 2019

Fix IDE0066 across corefx (dotnet/corefx#40288)
* Fix IDE0066 across corefx

"Fix all", but then manual tweaking to accommodate places the rule doesn't work quite right.

* Address PR feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.