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

Update 'WinFx' -> 'WinFX' casing #5346

Closed
wants to merge 4 commits into from
Closed

Update 'WinFx' -> 'WinFX' casing #5346

wants to merge 4 commits into from

Conversation

Nirmal4G
Copy link
Contributor

@Nirmal4G Nirmal4G commented May 10, 2020

As the title suggests, This PR renames WinFx to WinFX where possible.

I've separated out the white-space changes to a separate commit.
Only files that were touched with WinFX casing changes are updated with formatting changes.
If it's not possible to merge and/or if you want the line & space changes applied to the whole repo, please let me know, I'll separate these into a new PR.

Related: dotnet/wpf#2975

The casing could also be updated for WPF (.NET Framework) targets and sources.

Nirmal4G added 4 commits May 8, 2020 21:34
'WinFX' is the correct casing as observed in the .NET CLR 2 frameworks
'WinFX' is the correct casing as observed in the .NET CLR 2 frameworks
In project files, scripts, comments, etc...
for only previously changed files (in this Patch tree)
Conforming to the repository's EditorConfig file
@Nirmal4G Nirmal4G marked this pull request as ready for review May 10, 2020 21:58
@Nirmal4G Nirmal4G changed the title Fix 'WinFx' -> 'WinFX' casing Update 'WinFx' -> 'WinFX' casing May 10, 2020
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Since Microsoft.WinFx.targets shipped with .NET Framework and is unlikely to be renamed/recase, I think we should match its shipped on-disk case, and propagate that case to our local shim copy. As a result, I don't think we should take this PR (even though the casing you propose is better to my eyes, too).

@Nirmal4G
Copy link
Contributor Author

Nirmal4G commented May 11, 2020

@rainersigwald

even though the casing you propose

I'm not proposing a new case change. NETCLR 2 msbuild tools shipped with WinFX correct casing. Somehow NETCLR 4 has changed the case into WinFx. I suspect automated tools.

I don't think we should take this PR

These targets still have the correct casing though!

<Import Project="$(MSBuildToolsPath)\Microsoft.WinFX.targets" Condition="'$(TargetFrameworkVersion)' != 'v2.0' and '$(TargetCompactFramework)' != 'true' and Exists('$(MSBuildToolsPath)\Microsoft.WinFX.targets')"/>

<Import Project="$(MSBuildToolsPath)\Microsoft.WinFX.targets" Condition="'$(TargetFrameworkIdentifier)' == 'Silverlight' and Exists('$(MSBuildToolsPath)\Microsoft.WinFX.targets')"/>

.NET Framework is Windows only and Case in-sensitive. Even though .NET Framework has high bar for compat, this doesn't seem to be a breaking change. This PR just matches them. I think It's OK to merge though.

So, Can I ask Why?

@rainersigwald
Copy link
Member

I don't know why it changed from .NET 3.5 to .NET 4.something. I agree that it shouldn't have--but at this point changing it back is not worth much and might break someone.

I took a look around and this file is present in Mono installs on macOS (at least), so it can be present on a case-sensitive filesystem. Combined with the low impact of the change I don't think we should take this.

Thanks for trying to clean this up!

@Nirmal4G Nirmal4G deleted the hotfix/fix-WinFX-casing branch June 13, 2020 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants