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 | DbConnectionOptions (The Rest Of It)#3388

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

Conversation

benrr101
Copy link
Contributor

Description

In the previous PR, we moved the debug-only code for DbConnectionOptions into its own partial class and renamed the remaining partials to follow the merge pattern (ie, *.netcore.cs and *.netfx.cs). This PR completes the process by moving the platform-specific partials into the common DbConnectionOptions.cs file. The code is wrapped in pre-processor directives to ensure they are only compiled on the appropriate platforms.

Issues

Continues work towards#1261

Testing

Code has moved but the functionality has not changed. Project still builds, and CI should be enough for validation.

@CopilotCopilotAI review requested due to automatic review settingsJune 3, 2025 16:14
@benrr101benrr101 added the Common Project 🚮Things that relate to the common project project labelJun 3, 2025
@benrr101benrr101 requested a review froma team as acode ownerJune 3, 2025 16:14
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Pull Request Overview

This PR consolidates the platform-specific partial classes for DbConnectionOptions into a single common file using preprocessor directives, removing the need for separate .netfx and .netcore files. Key changes include merging platform-specific code into DbConnectionOptions.cs, removing the obsolete partial files, and updating the project files to reflect these deletions.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.csMerged platform-specific code with preprocessor directives; added a TODO comment regarding parser extraction.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.netfx.csRemoved as its contents are now merged into the common DbConnectionOptions.cs file.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/Common/ConnectionString/DbConnectionOptions.netcore.csRemoved as its contents are now merged into the common DbConnectionOptions.cs file.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient.csprojUpdated to remove reference to the .netfx partial class file.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient.csprojUpdated to remove reference to the .netcore partial class file.

mdaigle
mdaigle previously approved these changesJun 3, 2025
Copy link
Contributor

@mdaiglemdaigle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

From a pure merge perspective, it looks good to me. Interested in some of the additional improvements that@edwardneal has suggested.

edwardneal reacted with thumbs up emoji
@codecovCodecov
Copy link

codecovbot commentedJun 3, 2025•
edited
Loading

Codecov Report

Attention: Patch coverage is80.28169% with14 lines in your changes missing coverage. Please review.

Project coverage is 68.20%. Comparing base(e649a7c) to head(30d7989).
Report is 4 commits behind head on main.

Files with missing linesPatch %Lines
...ata/Common/ConnectionString/DbConnectionOptions.cs80.28%14 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##             main    #3388      +/-   ##==========================================- Coverage   68.33%   68.20%   -0.14%==========================================  Files         301      299       -2       Lines       65631    65406     -225     ==========================================- Hits        44849    44608     -241- Misses      20782    20798      +16
FlagCoverage Δ
addons92.58% <ø> (ø)
netcore68.78% <100.00%> (-3.83%)⬇️
netfx70.65% <77.77%> (+3.66%)⬆️

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.

🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paulmedynski
paulmedynski previously approved these changesJun 10, 2025
@benrr101benrr101 dismissed stale reviews frompaulmedynski andmdaigle via30d7989June 11, 2025 23:41
@benrr101benrr101 merged commit2385ca7 intomainJun 12, 2025
251 checks passed
@benrr101benrr101 deleted the dev/russellben/merge/dbconnectionoptions-platform-specific branchJune 12, 2025 16:42
@paulmedynskipaulmedynski added this to the6.1-preview2 milestoneJun 23, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@edwardnealedwardnealedwardneal left review comments

Copilot code reviewCopilotCopilot left review comments

@mdaiglemdaiglemdaigle approved these changes

@paulmedynskipaulmedynskipaulmedynski approved these changes

Assignees
No one assigned
Labels
Common Project 🚮Things that relate to the common project project
Projects
None yet
Milestone
6.1-preview2
Development

Successfully merging this pull request may close these issues.

4 participants
@benrr101@mdaigle@paulmedynski@edwardneal
[8]ページ先頭
©2009-2025 Movatter.jp