-
Notifications
You must be signed in to change notification settings - Fork 497
In <PackageReference Condition/>
docs, mention IsTargetFrameworkCompatible
#3432
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
base: main
Are you sure you want to change the base?
Conversation
Learn Build status updates of commit 09c384b: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
@@ -170,6 +170,20 @@ Conditions can also be applied at the `ItemGroup` level and will apply to all ch | |||
</ItemGroup> | |||
``` | |||
|
|||
When you have many targets, it may be better to match ranges of TFMs, in which case you can use `IsTargetFrameworkCompatible`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we link to https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-targetframework-and-targetplatform-functions as well?
https://github.com/NuGet/docs.microsoft.com-nuget?tab=readme-ov-file#links
It's worth noting that the more complicated the conditions are, the more challenging it is for any tooling to update these correctly.
I'd consider these to be advanced scenarios, so my suggestion would be to call them out as such.
Learn Build status updates of commit 8e2c834: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
I've just noticed the third example doesn't have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution.
Sorry for not doing as thorough of a review the first go.
@@ -170,6 +170,20 @@ Conditions can also be applied at the `ItemGroup` level and will apply to all ch | |||
</ItemGroup> | |||
``` | |||
|
|||
When you have many targets, it may be better to match ranges of TFMs, in which case you can use [`IsTargetFrameworkCompatible`](/visualstudio/msbuild/property-functions#msbuild-targetframework-and-targetplatform-functions): | |||
```xml | |||
<ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3 examples are pretty equivalent.
They're all showing the usage of IsTargetFrameworkCompatible with negation.
I think it'll be easier for the reader if they were to see one example.
```xml | ||
<ItemGroup> | ||
<!-- reference 8.0 System.Text.Json when targeting things older than .NET 8 --> | ||
<PackageReference Include="System.Text.Json" Version="8.0.5" Condition=" !$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0')) " /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably me overthinking it because of https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#prunepackagereference, but the idea is that we generally want to make sure people are not referencing platform packages unless they're absolutely required.
For example, there's many System.Text.Json references when customers target .NET and that's unnecessary.
So I'm thinking that we can maybe use a Contoso named package, or if we're sticking with System.Text.Json we can use: GetTargetFrameworkIdentifier.
Maybe we end up with 2 examples, 1 using IsTargetFrameworkCompatible
, while the other one uses GetTargetFrameworkIdentifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in $([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) != '.NETCoreApp'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the .NET SDK sets property TargetFrameworkIdentifier
, and MSBuild evaluates properties top to bottom before items, doing a TFI condition is easier by using Condition=" '$(TargetFrameworkIdentifier)' != '.NETCoreApp' "
, rather than calling the instrinsic function and passing the target framework as an argument.
examples based on dotnet/docs#45462