Rename cross-targeting to multi-targeting #1471
Conversation
| --> | ||
| <PropertyGroup Condition="'$(TargetFrameworks)' != '' and '$(TargetFramework)' == ''"> | ||
| <IsCrossTargetingBuild>true</IsCrossTargetingBuild> | ||
| <IsMultiTargetingBuild>true</IsMultiTargetingBuild> |
There was a problem hiding this comment.
We may need to set both as compat shim as discussed in email,
| @@ -20,7 +20,7 @@ | |||
| namespace Microsoft.Build.CommandLine | |||
| { | |||
| /// <summary> | |||
| /// This is the Out-Of-Proc Task Host for supporting Cross-Targeting tasks. | |||
| /// This is the Out-Of-Proc Task Host for supporting Multi-Targeting tasks. | |||
There was a problem hiding this comment.
This one actually means cross-compiling, please revert.
There was a problem hiding this comment.
oops. guess that's another point in favor of this change.
| @@ -10,15 +10,7 @@ Copyright (C) Microsoft Corporation. All rights reserved. | |||
| *********************************************************************************************** | |||
| --> | |||
|
|
|||
| <!-- These targets are deprecated, forward to new targets --> | |||
There was a problem hiding this comment.
Is this intended as a temporary measure so we can update the SDK gradually? Since we haven't shipped an official release with these targets I hope it'd be ok to just rename them atomically.
|
@rainersigwald @dsplaisted I'm actually now wondering if we should rip the cross-targeting stuff out of msbuild. Now that the SDK has more control over imports than after common props & targets, it could handle all of it by itself without a built-in feature in msbuild. |
|
(The non x-targeting -> x-targeting P2P resolution would need to remain in msbuild, though). |
|
@nguerrera I'm in favor. |
mhutch
force-pushed
the
rename-cross-targeting
branch
2 times, most recently
from
cdb887c to
36ca8f2
Compare
|
I think I addressed all the feedback. |
| file source=$(X86BinPath)Microsoft.CSharp.CrossTargeting.targets | ||
| file source=$(X86BinPath)Microsoft.CSharp.targets | ||
| file source=$(X86BinPath)Microsoft.NetFramework.CurrentVersion.props | ||
| file source=$(X86BinPath)Microsoft.NetFramework.CurrentVersion.targets | ||
| file source=$(X86BinPath)Microsoft.NetFramework.props | ||
| file source=$(X86BinPath)Microsoft.NetFramework.targets | ||
| file source=$(X86BinPath)Microsoft.VisualBasic.CurrentVersion.targets | ||
| file source=$(X86BinPath)Microsoft.VisualBasic.MultiTargeting.targets |
There was a problem hiding this comment.
These changes needs to go in the amd64 folder as well (further down in the file). Use the same file source, just needs to be dropped to amd64.
mhutch
force-pushed
the
rename-cross-targeting
branch
from
36ca8f2 to
1a514bc
Compare
|
OK, i think I addressed all remaining feedback. |
|
Sorry to not be able to take this, at this point I think we have to close. This wouldn't meet the bar for this release, and I don't think it's worth changing at this point. If you think this is still worth discussing for Update 1 time-frame please re-open and we can discuss. Thanks! |
|
@AndyGerlicher since this is still tracking for dotnet/sdk#486 and the NuGet side was merged already, i think it makes sense to reopen, however I don't have permissions to do so |
|
We didn't get this past the bar for RTW and now it would be a breaking change from RTW. I think we're stuck with the "cross" name. |
|
But the PR supported back-compat 😞 |
|
@rainersigwald if it's not a breaking change can we consider taking this? |
|
Bump! Multi-targeting has become the generally accepted term for this feature and it's odd that the targets use a different terminology in a fairly visible way. |
|
Is this in already? This renaming has already happened on NuGet as well as asp.net (see NuGet/Home#4098) |
|
We didn't have any plan for this. Last I heard this was discussed internally and decided against back when it was closed. I followed the issue you linked (and all the items that mention that), but I couldn't find anything that referred to something that was merged (as opposed to just closed). You're sure this already happened? |
|
@AndyGerlicher NuGet/NuGet.Client@efae789 mentions |
per dotnet/sdk#486