- Notifications
You must be signed in to change notification settings - Fork311
Merge | Netfx SMI Classes#3326
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ject, some very light cleanup
… project, Removing obsolete code
…ame to SmiXetterAccessMap.cs
…erAction.netfx.cs in common project
codecovbot commentedMay 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #3326 +/- ##==========================================- Coverage 67.74% 65.10% -2.64%========================================== Files 298 298 Lines 65536 65469 -67 ==========================================- Hits 44397 42624 -1773- Misses 21139 22845 +1706
Flags with carried forward coverage won't be shown.Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I'm pretty sure that everything here will be deleted when the context connection PR set (#2996 and its follow-ups) is finished. Every path here is already unused because we block context connections in the connection string. Do you still want to merge the files? I completely agree with removing the event sink code from SqlCommand. I think the original SQLCLR methodology had a set of custom event sinks and a custom SmiLink derivative in the SqlAccess assembly, so it's used to support that. |
nooooooooooooooooooooooooooooooooooooooooooooooooooooooo 😭 |
// SqlDbTypes as rows | ||
// bool, byte, bytes, chars, strng, int16, int32, int64, singl, doubl, sqldec, date, guid, varmd, Xetr, time, dtost | ||
/*BigInt*/ { _, _, _, _, _, _, _, X, _, _, _, _, _, _, _, _, _, }, | ||
/*Binary*/ { _, _, X, _, _, _, _, _, _, _, _, _, _, _, _, _, _, }, |
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 some crazy stuff 😟
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.
What's crazier - a lot of this is just going to disappear in a later PR :)
@@ -405,7 +405,7 @@ internal override void ParameterAvailable(SmiParameterMetaData metaData, SmiType | |||
private SmiContext _smiRequestContext; // context that _smiRequest came from | |||
private CommandEventSink _smiEventSink; | |||
privateSmiEventSink_DeferedProcessing _outParamEventSink; | |||
privateSmiEventSink_DeferredProcessing _outParamEventSink; |
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.
Heh, I changed this on another branch of mine as well. Shouldn't conflict.
/// <summary> | ||
/// Formal encoding of SMI's metadata-to-ITypedSetter/-from-ITypedGetter validity rules | ||
/// </summary> | ||
internal partial class SmiXetterAccessMap |
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.
What's a Xetter?
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.
Afaik it's [G|S]etter - so this has both getters and setters
b3a72fc
intomainUh oh!
There was an error while loading.Please reload this page.
Description: This PR is a bit bigger than usual, but it's mostly cut/paste of classes from the netfx project to the common project. There are a few additional changes going on in this PR though that are worth calling out:
SmiContextFactory
to just make sure it's following some style guidelinesSmiEventStream.Dispose
obsolete (since it is notated to be obsolete and only throws)SqlCommand
, but there are no classes that inherit from it. All the virtual code paths overridden in it throw, so any codepath that uses it will always throw. I suspect it is ultimately not used. I will investigate if this entire codepath can be deleted.TriggerAction
enum definitions derive directly fromEMDEventType
enum definitions, which seems completely pointless to me. So, I rolled theEMDEventType
values into theTriggerAction
enum and deletedEMDEventType
.SmiXetterAccessMap
classes were combined together with#if NETFRAMEWORK
.SmiEventSink_DeferedProcessing
toSmiEventSink_DeferredProcessing
(typo in Deferred)Testing: Project builds locally, will wait for CI to call it good.