Adding transitive project references based on target framework #15124
Conversation
src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToReferenceAProject.cs
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageDependencies.cs
Outdated
Show resolved
Hide resolved
| if (string.IsNullOrEmpty(group.FrameworkName) || string.IsNullOrEmpty(frameworkAlias) || group.FrameworkName.Equals(frameworkAlias) || | ||
| NuGetUtils.ParseFrameworkName(group.FrameworkName.Split('/').First()).DotNetFrameworkName.Equals(NuGetUtils.ParseFrameworkName(frameworkAlias).DotNetFrameworkName)) |
There was a problem hiding this comment.
Do we need to check if group.FrameworkName or frameworkAlias are empty? Shouldn't they always have values?
Also, I think that eventually we should not need to parse these to compare them. I think that eventually the dependency group's FrameworkName should match the target framework alias exactly. However, currently it doesn't. For now I would factor out the match calculation into a helper method to make it a bit easier to change later.
@nkolev92 @zkat Do you agree that when NuGet supports generalized target framework aliasing, that the key for the dependency group would change to be the target framework alias? Is there an issue for that we should link?
There was a problem hiding this comment.
The target framework isn't marked as a required input for one of the tasks that uses this extension, so this guards against the case where it isn't provided. Without it we're breaking several existing tests.
There was a problem hiding this comment.
Do you agree that when NuGet supports generalized target framework aliasing, that the key for the dependency group would change to be the target framework alias? Is there an issue for that we should link?
Not sure what the key is in this case. Can you please elaborate a bit?
Probably related NuGet/Home#5154
|
@dsplaisted does this look okay to you or do you have any more feedback? |
sfoslund
added
auto-merge
|
Hello @sfoslund! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
Fixes #1227