ref: replace concrete TargetFramework string comparisons with TargetFrameworkIdentifier/Version properties#5240
Merged
Merged
Conversation
…rameworkIdentifier/TargetFrameworkVersion properties
Replace `$(TargetFramework) == 'net8.0'` style conditions with
`$(TargetFrameworkIdentifier) == '.NETCoreApp' And $(TargetFrameworkVersion) == 'v8.0'`
equivalents across src, test, and samples project files, as recommended by
the MSBuild best practices guidance. Also replaces `TargetFramework.Contains('-ios')`
with `$(TargetPlatformIdentifier) == 'ios'`.
Closes #2657
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: James Crosswell <james.crosswell@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5240 +/- ##
==========================================
+ Coverage 74.12% 74.21% +0.09%
==========================================
Files 508 509 +1
Lines 18282 18386 +104
Branches 3574 3600 +26
==========================================
+ Hits 13551 13645 +94
- Misses 3861 3867 +6
- Partials 870 874 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
jamescrosswell
commented
May 17, 2026
A bunch of these were unnecessarily specific, simply because historically it was easier to do an exact match than fuzzy matching. Checking the identifier and the versions separately, we can drop the version checks whenever we're checking for netfx and often times when checking for netstandard as well (e.g. when we only target on version of netstandard). Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
jamescrosswell
marked this pull request as ready for review
May 18, 2026 22:14
jamescrosswell
added
the
skip-changelog
Flash0ver
reviewed
Jul 7, 2026
Extend the TargetFramework -> TargetFrameworkIdentifier/Version conversion to the remaining conditions raised in review, and make the approach reliable everywhere. Pre-compute TargetFrameworkIdentifier/TargetFrameworkVersion in Directory.Build.props (exactly as the SDK does, including the 'v' version prefix). The SDK only sets these late in its own targets, so they were empty during early <PropertyGroup> evaluation - computing them early lets us use the property-based syntax consistently in both <PropertyGroup> and <ItemGroup> conditions instead of $(TargetFramework) string checks. Also: - Convert the remaining net4/netstandard/net48 conditions across src, test and samples. - Drop incidental .NET Framework version pins; the specific version was never required. - Simplify the native-AOT upload gate to the existing $(_SentryIsNet8OrGreater) helper, which also correctly excludes net5/6/7 (the previous list missed net5). - Remove the dead RS0017 NoWarn (the analyzer that emitted it was removed in #1313). - Use a clearer negated form for the Serilog netstandard2.0 package condition. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Flash0ver
approved these changes
Jul 9, 2026
Flash0ver
reviewed
Jul 9, 2026
The root Directory.Build.props already computes TargetPlatformIdentifier early (before the project body), and this project imports it, so the local recompute in the arm64 RuntimeIdentifier group was a no-op reassignment of the same value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace hardcoded
$(TargetFramework)string comparisons in.csprojfiles with the more scalable$(TargetFrameworkIdentifier)+$(TargetFrameworkVersion)property checks, as recommended by MSBuild best practices.'$(TargetFramework)' == 'net8.0'→'$(TargetFrameworkIdentifier)' == '.NETCoreApp' And '$(TargetFrameworkVersion)' == 'v8.0''$(TargetFramework)' == 'net48'→'$(TargetFrameworkIdentifier)' == '.NETFramework' And '$(TargetFrameworkVersion)' == 'v4.8''$(TargetFramework)' == 'netstandard2.0'→'$(TargetFrameworkIdentifier)' == '.NETStandard' And '$(TargetFrameworkVersion)' == 'v2.0'$(TargetFramework.Contains('-ios'))→'$(TargetPlatformIdentifier)' == 'ios'14 files updated across
src/,test/, andsamples/.#skip-changelog
Closes #2657