- Notifications
You must be signed in to change notification settings - Fork311
Merge | SqlClientPermission#3262
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
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.
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Files not reviewed (1)
- src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj: Language not supported
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientPermission.netfx.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientPermission.netfx.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientPermission.netfx.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientPermission.netfx.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
codecovbot commentedApr 10, 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 #3262 +/- ##==========================================+ Coverage 72.98% 73.03% +0.04%========================================== Files 299 299 Lines 57215 57194 -21 ==========================================+ Hits 41760 41770 +10+ Misses 15455 15424 -31
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:
|
| public override IPermission CreatePermission() | ||
| { | ||
| return new SqlClientPermission(this); | ||
| } |
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.
| publicoverrideIPermissionCreatePermission() | |
| { | |
| returnnewSqlClientPermission(this); | |
| } | |
| publicoverrideIPermissionCreatePermission()=>newSqlClientPermission(this); |
| private bool _IsUnrestricted | ||
| { | ||
| get |
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.
get => base.IsUnrestricted();
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.
Personally, I like function bodies enclosed in braces. And in this case, it matches the format of the sibling setter. The => notation increases code density, which isn't necessarily a good thing. Personal preference I guess.
| public override IPermission Copy() | ||
| { | ||
| return new SqlClientPermission(this); | ||
| } |
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.
| publicoverrideIPermissionCopy() | |
| { | |
| returnnewSqlClientPermission(this); | |
| } | |
| publicoverrideIPermissionCopy()=>newSqlClientPermission(this); |
| internal NameValuePermission CopyNameValue() | ||
| { | ||
| return new NameValuePermission(this); | ||
| } |
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.
| internalNameValuePermissionCopyNameValue() | |
| { | |
| returnnewNameValuePermission(this); | |
| } | |
| internalNameValuePermissionCopyNameValue()=>newNameValuePermission(this); |
| private bool _IsUnrestricted | ||
| { | ||
| get |
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.
Personally, I like function bodies enclosed in braces. And in this case, it matches the format of the sibling setter. The => notation increases code density, which isn't necessarily a good thing. Personal preference I guess.
| } | ||
| string unrestrictedValue = securityElement.Attribute(XmlStr._Unrestricted); | ||
| _IsUnrestricted = unrestrictedValue != null && bool.Parse(unrestrictedValue); |
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.
Just FYI, but this seemingly benign assignment can (surprisingly) throw reflection exceptions, which we don't document. In fact, the docs don't claim this function will throw anything!
516b402 intomainUh oh!
There was an error while loading.Please reload this page.
Description: This change is a bit heavier than it should be because of reasons...
Testing: For the most past, there are no functional changes. Code is moved around. CI should validate.