Skip to content

Conversation

benrr101
Copy link
Contributor

Description: This is the easiest one so far - literally moving the SqlColumnEncryption*.Unix classes to the common project. The files were wrapped in an #if NET to make sure they're only applied to netcore. The suffixes were updated to ".netcore.Unix" - I'm not entirely sure the best way to approach this since the class only applies to Unix, which implicitly means it only applies to netcore.

Testing: The files were cut and pasted, there was no code changes.

@benrr101 benrr101 added the Common Project 🚮 Things that relate to the common project project label Apr 25, 2025
@benrr101 benrr101 requested review from a team and Copilot April 25, 2025 22:25
Copy link
Contributor

@Copilot Copilot AI 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 moves the SqlColumnEncryption* classes targeting Unix from platform-specific locations to the common project and wraps them in a preprocessor directive to ensure they are only applied to .NET Core builds. Key changes include:

  • Wrapping Unix-specific code in "#if NET" preprocessor directives.
  • Renaming files to include the ".netcore.Unix" suffix.

Reviewed Changes

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

File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCspProvider.netcore.Unix.cs Added "#if NET" wrapper and corresponding "#endif".
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCngProvider.netcore.Unix.cs Added "#if NET" wrapper and corresponding "#endif".
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCertificateStoreProvider.netcore.Unix.cs Added "#if NET" wrapper and corresponding "#endif".
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj: Language not supported
Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCspProvider.netcore.Unix.cs:5

  • [nitpick] Consider revisiting the preprocessor condition here. Since the file is exclusively for Unix on .NET Core, using a more specific conditional (e.g., #if NETCOREAPP) might improve clarity and maintainability.
#if NET

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCngProvider.netcore.Unix.cs:5

  • [nitpick] Consider revisiting the preprocessor condition here. Since the file is exclusively for Unix on .NET Core, using a more specific conditional (e.g., #if NETCOREAPP) might improve clarity and maintainability.
#if NET

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCertificateStoreProvider.netcore.Unix.cs:5

  • [nitpick] Consider revisiting the preprocessor condition here. Since the file is exclusively for Unix on .NET Core, using a more specific conditional (e.g., #if NETCOREAPP) might improve clarity and maintainability.
#if NET

@benrr101 benrr101 added this to the 6.1-preview2 milestone Apr 29, 2025
Copy link

codecov bot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.20%. Comparing base (965e5ff) to head (f2737d9).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3305      +/-   ##
==========================================
- Coverage   67.74%   64.20%   -3.55%     
==========================================
  Files         298      298              
  Lines       65536    65536              
==========================================
- Hits        44397    42076    -2321     
- Misses      21139    23460    +2321     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore 68.24% <ø> (-3.91%) ⬇️
netfx 64.99% <ø> (-1.17%) ⬇️

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.

@benrr101 benrr101 merged commit 0a8a2ba into main May 6, 2025
251 checks passed
@benrr101 benrr101 deleted the dev/russellben/merge/sqlcolumnencryption-unix branch May 6, 2025 17:33
Sign up for free to 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.

3 participants