Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Merge | SqlFileStream (Opt 1)#2898

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

Merged

Conversation

benrr101
Copy link
Contributor

Description: This PR aims to mergeSqlFileStream from the netfx and netcore projects. The strategy for this merge is to use the Interop.* libraries from the netcore project to enable merging the classes. Therefore, the code for this class is mostly based on the netcore implementation with a handful of#if NETFRAMEWORK blocks to add overrides for netfx behavior.

The commits are broken up into fairly digestible chunks that explain each step along the process. I'd recommend reviewing the PR by stepping through the commits and seeing if you agree with the changes. Nevertheless, this PR has a lot of code cleanup changes at the end that make the overall PR fairly large. In anticipation of pushback on this, I will offer a separate PR that only contains the merge changes. If that one is accepted, I'll still send out the cleanup change on a separate PR. But, I think this option would be faster overall.

Testing: The existing manual tests for SqlFileStream were used to validate behavior.

edwardneal reacted with rocket emoji
@codecovCodecov
Copy link

codecovbot commentedOct 7, 2024
edited
Loading

Codecov Report

Attention: Patch coverage is0% with279 lines in your changes missing coverage. Please review.

Project coverage is 72.24%. Comparing base(39c4604) to head(967e888).
Report is 3 commits behind head on main.

Files with missing linesPatch %Lines
...c/Microsoft/Data/SqlTypes/SqlFileStream.Windows.cs0.00%279 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##             main    #2898      +/-   ##==========================================+ Coverage   71.92%   72.24%   +0.32%==========================================  Files         294      291       -3       Lines       60342    59769     -573     ==========================================- Hits        43398    43180     -218+ Misses      16944    16589     -355
FlagCoverage Δ
addons92.90% <ø> (ø)
netcore75.81% <0.00%> (-0.02%)⬇️
netfx70.59% <0.00%> (+0.13%)⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

Seems like a lot of hassle for little benefit :/
* Fix annoyance with failing unit test setup
* Use variable versions* Remove output type from common project
@benrr101benrr101force-pushed thecommon-sqlfilestream-smallercleanup branch from9e157a5 toefad644CompareOctober 10, 2024 21:11
@cheenamalhotracheenamalhotra added this to the6.0-preview2 milestoneOct 15, 2024
@cheenamalhotra
Copy link
Member

Can you please take a look at conflicts, thanks!

@benrr101benrr101 merged commit2e163b2 intodotnet:mainOct 25, 2024
124 checks passed
@benrr101benrr101 deleted the common-sqlfilestream-smallercleanup branchOctober 25, 2024 19:55
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@edwardnealedwardnealedwardneal left review comments

@mdaiglemdaiglemdaigle approved these changes

@cheenamalhotracheenamalhotracheenamalhotra approved these changes

@VladimirReshetnikovVladimirReshetnikovAwaiting requested review from VladimirReshetnikov

@saurabh500saurabh500Awaiting requested review from saurabh500

@David-EngelDavid-EngelAwaiting requested review from David-Engel

@samsharma2700samsharma2700Awaiting requested review from samsharma2700

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.0-preview2
Development

Successfully merging this pull request may close these issues.

4 participants
@benrr101@cheenamalhotra@mdaigle@edwardneal

[8]ページ先頭

©2009-2025 Movatter.jp