Fix bunch of annontations #1959
Conversation
| @@ -1232,6 +1237,11 @@ override public IEnumerator GetEnumerator() | |||
| } | |||
|
|
|||
| /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/GetFieldType/*' /> | |||
| #if NET6_0_OR_GREATER | |||
| [SuppressMessage("ReflectionAnalysis", "IL2093:MismatchOnMethodReturnValueBetweenOverrides", | |||
There was a problem hiding this comment.
What kind of annotations? and were they shipped in 7?
There was a problem hiding this comment.
I think issue was that ref assembly was missing this annotations.
https://github.com/dotnet/runtime/blame/main/src/libraries/System.Data.Common/ref/System.Data.Common.cs#L2323-L2324
and yes, it was fixed in .NET 7 (or even .NET 8)
| @@ -942,6 +964,9 @@ ParameterDirection direction | |||
| byte scale, | |||
| long localeId, | |||
| SqlCompareOptions compareOptions, | |||
| #if NET6_0_OR_GREATER | |||
There was a problem hiding this comment.
I don't like that this isn't clear that it applies to the parameter following the #endif but i can't see another way to format the code to make it clear :(
There was a problem hiding this comment.
Yeah, that's unfortunate.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1959 +/- ##
==========================================
+ Coverage 69.89% 70.64% +0.74%
==========================================
Files 305 305
Lines 61944 61977 +33
==========================================
+ Hits 43297 43783 +486
+ Misses 18647 18194 -453
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
kant2002
force-pushed
the
kant/fix-annotations
branch
from
ba08c5c
to
2b2b4b8
Compare
|
@lcheunglci May you take a look? |
|
@lcheunglci I still have hopes I catch your attention, and move whole thing a bit forward 😄 |
Sorry I haven't been as active lately. Perhaps, @JRahnama @DavoudEshtehari or @Kaur-Parminder when they have a moment could also take a look. |
|
Okay. Sorry to bother, probably misinterpret your like |
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
Outdated
Show resolved
Hide resolved
Kaur-Parminder
requested review from
Kaur-Parminder,
DavoudEshtehari and
arellegue
|
/azp run CI-SqlClient |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kant2002
force-pushed
the
kant/fix-annotations
branch
from
3654e73
to
6565ab8
Compare
|
It looks like failure are from unreliable tests? I have hard time understand how annotations can be the cause. |
The failure is not related to your changes. I am going to rerun them one more time to see the outcome. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
When not ignoring trimming anotations does not affect runtime behavoir of the code, but affect dev workflow. For developer who consume SqlClient it will mean that it can use this library with trimming and know that if analyzer do not produce warnings, then they are safe. Right now if you use trimming/NativeAOT you should pray that SqlClient is trimming friendly (I would say it is, but now we will have proof for that) |
Actually remove some trimming issues completely.