Skip to content

Merge/Cleanup | GreenMethods - #3254

Merged
paulmedynski merged 2 commits into
mainfrom
dev/russellben/merge/greenmethods
Apr 8, 2025
Merged

Merge/Cleanup | GreenMethods#3254
paulmedynski merged 2 commits into
mainfrom
dev/russellben/merge/greenmethods

Conversation

@benrr101

Copy link
Copy Markdown
Contributor

Description: There was an oddly named GreenMethods in the common namespace for the netfx project. Upon inspection, the code was only being used in SqlClientFactory, so I rolled that into the SqlClientFactory. Not only that, but I simplified the implementation quite a bit by using Lazy objects (which was sort of called for in the original comments). A permission attribute about reflection was removed in the process and if it turns out this was super important, I can roll these changes back to a more traditional merge.

As such, the GreenMethods class is removed 🚮

@benrr101benrr101 added the Common Project 🚮 Things that relate to the common project project label Apr 2, 2025
@benrr101
benrr101 requested review from a team and CopilotApril 2, 2025 19:36

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 removes the unused GreenMethods class and integrates its functionality directly into SqlClientFactory with a simplified lazy-loading implementation. Key changes include:

  • Elimination of the GreenMethods class.
  • Incorporation of lazy initialization for provider service types within SqlClientFactory.
  • Simplification of error message documentation and reduction of redundant reflection code.

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

FileDescription
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientFactory.csMerges GreenMethods functionality into SqlClientFactory and refactors service resolution logic.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/GreenMethods.csRemoves the deprecated GreenMethods class as it is no longer needed.
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj: Language not supported

@codecov

codecovBot commented Apr 2, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.94%. Comparing base (4c7219d) to head (6f069da).
Report is 5 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #3254 +/- ##
==========================================
+ Coverage 72.82% 72.94% +0.12% 
==========================================
Files 298 297 -1 Lines 59614 59614 ==========================================
+ Hits 43411 43483 +72 + Misses 16203 16131 -72 
FlagCoverage Δ
addons92.58% <ø> (ø)
netcore75.35% <100.00%> (+0.04%)⬆️
netfx71.53% <100.00%> (+0.15%)⬆️

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.

@mdaiglemdaigle self-assigned this Apr 3, 2025
@paulmedynski
paulmedynski merged commit 2bb0dc7 into mainApr 8, 2025
@paulmedynski
paulmedynski deleted the dev/russellben/merge/greenmethods branch April 8, 2025 15:16
@cheenamalhotracheenamalhotra added this to the 6.1-preview1 milestone Apr 16, 2025
This was referenced Jul 25, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Common Project 🚮Things that relate to the common project project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@benrr101@mdaigle@paulmedynski@cheenamalhotra