Clean up Preprocessor Directives After NET6 Removal #2938
Conversation
mdaigle
added
the
Code Health 💊
mdaigle
changed the title
|
Looks like this includes your previous PR? |
@ErikEJ yes, this is building on the previous PR. I'll move it out of draft when the previous PR merges. |
| <TargetGroup Condition="$(TargetGroup) == ''">netcoreapp</TargetGroup> | ||
| <RuntimeIdentifier Condition="'$(TargetGroup)'=='netfx'">win</RuntimeIdentifier> | ||
| <RuntimeIdentifier Condition="'$(TargetGroup)'=='netfx' AND $(ReferenceType.Contains('Package')) AND !$(Platform.Contains('AnyCPU'))">win-$(Platform)</RuntimeIdentifier> | ||
| <DefineConstants Condition="'$(TargetGroup)'=='netfx'">$(DefineConstants);NETFRAMEWORK</DefineConstants> |
There was a problem hiding this comment.
These constants are already provided by the sdk.
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2938 +/- ##
=======================================
Coverage ? 72.21%
=======================================
Files ? 291
Lines ? 59709
Branches ? 0
=======================================
Hits ? 43120
Misses ? 16589
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| #if NET8_0_OR_GREATER | ||
| #if NET |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, I'm not too particular about it - I think we're going to have fun cleaning up all this if/when we drop netfx support. Sorry if I seemed like too much of a stick in the mud about it in your PRs 😬
mdaigle
changed the title
benrr101
left a comment
benrr101
left a comment
There was a problem hiding this comment.
Love it - it's not stomping on any of the changes I'm making, and it's good to remove these cases that are no longer necessary
| #if NET8_0_OR_GREATER | ||
| d.WriteTdsValue(data); | ||
| #else | ||
| SqlTypeWorkarounds.SqlDecimalExtractData(d, out data[0], out data[1], out data[2], out data[3]); |
There was a problem hiding this comment.
Can the type workarounds be removed now?
There was a problem hiding this comment.
A quick look says at least some of it can be removed. I'll leave it to @benrr101 because the type workarounds are split across netcore and netfx, so they'll likely be touched by the merge efforts.
|
LGTM |
5 participants
Building on #2927. That PR should merge first.
This PR includes a few different types of preprocesser directive cleanups:
#if NET6_0_OR_GREATERand#if NET8_0_OR_GREATER#if NETFRAMEWORK#if NETX_0_OR_GREATER#if NETX_0_OR_GREATERwith#if NET