ProjectReference protocol documentation updates #6892
Conversation
- Specify that `GetTargetFrameworks` is required for outer builds only - Fix some markdown formatting so that XML elements appear correctly - Remove a duplicated word
| These targets are all defined in `Microsoft.Common.targets` and are defined in Microsoft SDKs. You should only have to implement them yourself if you require custom behavior or are authoring a project that doesn't import the common targets. | ||
|
|
||
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` is required in the “outer” build. The other targets listed can be “inner” build only. | ||
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` and `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. |
There was a problem hiding this comment.
Please verify this change. My understanding is that GetTargetFrameworks is also part of the outer build, and that this sentence was not updated when the docs for the target were added below.
There was a problem hiding this comment.
I think GetTargetFrameworkProperties doesn't exist anymore? @dplaisted correct me if I'm wrong, but it looks like GetTargetFrameworkProperties may have been renamed to GetTargetFrameworks. I grepped through the sdk repo and couldn't find a target with that name. Here's the comment about this target in common targets:
<!--
============================================================
GetTargetFrameworkProperties
Overrridden by cross-targeting projects to return the set of
properties (in the form "key1=value1;...keyN=valueN") needed
to build it with the best target for the referring project's
target framework.
The referring project's $(TargetFrameworkMoniker) is passed
in as $(ReferringTargetFramework)
-->
<Target Name="GetTargetFrameworkProperties" />
This might just be dead code
There was a problem hiding this comment.
Yeah, so you're thinking this right?
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` and `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. | |
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. |
| These targets are all defined in `Microsoft.Common.targets` and are defined in Microsoft SDKs. You should only have to implement them yourself if you require custom behavior or are authoring a project that doesn't import the common targets. | ||
|
|
||
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` is required in the “outer” build. The other targets listed can be “inner” build only. | ||
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` and `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. |
There was a problem hiding this comment.
Yeah, so you're thinking this right?
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` and `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. | |
| If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworks` are required in the “outer” build. The other targets listed can be “inner” build only. |
|
The change looks good, should we kill this off from common.currentversion.targets? We can combine it with this PR. It's been a while since we've stopped using this target. It's specifically used by the SDK and customers shouldn't be depending on it. Side note: Since this is a low pri PR, I'd like to update the doc with the recent setplatform changes before this merges. |
|
With the latest three commits I think this is good to merge. 2517690 might be divisive, but this is a target that's been unused by us for a while now. |
…errors when logging an error
|
Looks good to me. I don't have merge permissions, so merge at will. |
| The referring project's $(TargetFrameworkMoniker) is passed | ||
| in as $(ReferringTargetFramework) | ||
| --> | ||
| <Target Name="GetTargetFrameworkProperties" /> |
There was a problem hiding this comment.
Yeah, this makes me nervous. It doesn't help that much (one empty target is no big deal), and might cause breaks (if someone's calling it explicitly).
To be conservative we could minimize the comment to "Obsolete; present only for theoretical backward compatibility".
This reverts commit 2517690.
benvillalobos
added
the
merge-when-branch-open
…asloggederrors when logging an error" This reverts commit a5ceebf.
|
Somehow a commit snuck in from a different PR. Reverted it, tests should pass now. |
4 participants
Context
Minor documentation changes.
Changes Made
GetTargetFrameworksis required for outer builds onlyTesting
N/A
Notes
See PR comment.