-
Notifications
You must be signed in to change notification settings - Fork 315
Abstractions Package - C# project changes #3626
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Abstractions Package - C# project changes #3626
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.
Pull Request Overview
This PR introduces the initial structure for a Microsoft.Data.SqlClient.Extensions.Abstractions package as part of the Azure split work. The purpose is to create a shared abstractions package that will contain types common to MDS and its future extensions, with Azure being the first extension.
Key changes include:
- Creation of new Abstractions package structure with basic sample class and tests
- Reorganization of MSBuild targets for package generation
- Addition of dependency references to the new Abstractions package across MDS projects
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tools/targets/add-ons/GenerateAkvPackage.targets | New MSBuild target for generating AKV packages |
tools/targets/GenerateSqlServerPackage.targets | Removed old package generation target |
tools/targets/GenerateMdsPackage.targets | New MSBuild target for generating MDS packages |
tools/specs/Microsoft.Data.SqlClient.nuspec | Added dependency on Abstractions package |
tools/props/Versions.props | Reorganized version properties and added package-specific versioning |
src/Microsoft.Data.SqlClient/*/Microsoft.Data.SqlClient.csproj | Added conditional references to Abstractions package |
src/Microsoft.Data.SqlClient.sln | Added Abstractions project structure to solution |
src/Microsoft.Data.SqlClient.Extensions/Abstractions/* | New Abstractions package source files with sample class and tests |
src/Directory.Packages.props | Added Abstractions package version configuration |
src/Directory.Build.props | Added configuration for embedding debug symbols in packages |
build.proj | Updated build targets and dependencies for Abstractions package |
NuGet.config | Updated package source configuration and documentation |
BUILDGUIDE.md | Updated build documentation to include Abstractions package |
.editorconfig | Added additional file types for consistent formatting |
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj
Outdated
Show resolved
Hide resolved
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.
Adding commentary for reviewers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/azure-split #3626 +/- ##
====================================================
+ Coverage 66.13% 66.67% +0.54%
====================================================
Files 276 276
Lines 60765 60765
====================================================
+ Hits 40184 40515 +331
+ Misses 20581 20250 -331
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:
|
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.
Tracking my own review progress, and GitHub requires a comment.
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.
More commentary.
- Removed MDS package ref dependency on Abstractions until pipelines are ready. - Renamed AbstractionsPackage to Abstractions in targets. - Updated BUILDGUIDE based on project ref behaviour. - Fixing Abstractions package version specification throughout the different builds. - Minimal YAML pipeline changes to accomodate Abstractions package versioning. - Removed ReferenceType as a variable in all .nuspec files. - Updated Nuget package generation targets to provide appropriate dependent package versions.
96b8487
to
383a354
Compare
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.
More commentary.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj
Show resolved
Hide resolved
tools/specs/add-ons/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.nuspec
Show resolved
Hide resolved
tools/specs/add-ons/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.nuspec
Show resolved
Hide resolved
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.
I think in general I am ok with these changes
src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.csproj
Show resolved
Hide resolved
0bf8d66
- Fixed expansion of split expression in version numbers. - Removed addition of PDBs from <AllowedOutputExtensionsInPackageBuildOutputFolder>. - Removed MDS package ref dependency on Abstractions until pipelines are ready. - Renamed AbstractionsPackage to Abstractions in targets. - Updated BUILDGUIDE based on project ref behaviour. - Added feature branches to CI pipeline triggers. - Added missing/incomplete paths to the trigger. - Added dev/* branches to the CI triggers so PRs that target other dev/ branches can run CI. - Added missing MdsPackageVersion property to signed build pipeline job. - Commented-out failing NuGet tool installer task. - Re-ordered Guardian analysis step _before_ build step to avoid clobbering versioned DLLs. - Added MDS package version to AKV build/package steps. - Restored AKV nuspec ReferenceType property for AKV Official builds. - Explicitly building tooling before analysis. - Fixing validation steps to match XML props files. - Added PR automation triggers, and documented other pipeline sections. - Added ReferenceType throughout the MDS/AKV CI build steps. - Added NuGet.config update to main CI build step for Package reference mode. - Swapped Abstractions download and NuGet.config update to ensure packages/ exists before attempting to re-configure NuGet. - Clean target no longer removes packages/ - Uncommented package refs to Abstractions. - Added separate MDS and AKV project builds to support Package mode. - Added $ReferenceType$ property to MDS .nuspec like it is for AKV.
- Fixed expansion of split expression in version numbers. - Removed addition of PDBs from <AllowedOutputExtensionsInPackageBuildOutputFolder>. - Removed MDS package ref dependency on Abstractions until pipelines are ready. - Renamed AbstractionsPackage to Abstractions in targets. - Updated BUILDGUIDE based on project ref behaviour. - Added feature branches to CI pipeline triggers. - Added missing/incomplete paths to the trigger. - Added dev/* branches to the CI triggers so PRs that target other dev/ branches can run CI. - Added missing MdsPackageVersion property to signed build pipeline job. - Commented-out failing NuGet tool installer task. - Re-ordered Guardian analysis step _before_ build step to avoid clobbering versioned DLLs. - Added MDS package version to AKV build/package steps. - Restored AKV nuspec ReferenceType property for AKV Official builds. - Explicitly building tooling before analysis. - Fixing validation steps to match XML props files. - Added PR automation triggers, and documented other pipeline sections. - Added ReferenceType throughout the MDS/AKV CI build steps. - Added NuGet.config update to main CI build step for Package reference mode. - Swapped Abstractions download and NuGet.config update to ensure packages/ exists before attempting to re-configure NuGet. - Clean target no longer removes packages/ - Uncommented package refs to Abstractions. - Added separate MDS and AKV project builds to support Package mode. - Added $ReferenceType$ property to MDS .nuspec like it is for AKV.
Overview
As part of the Azure split work, a new Abstractions package is being created that contains types shared between MDS and its future extensions (Azure will be the first). This PR contains the C# project changes that create that package (with no meaningful content) to setup the MDS dependency.
This supercedes the original PRs #3471 and #3567.
This is depended on by PR #3628, which contains all of the pipeline changes to get the Abstractions package built and tested for CI, and packaged for official release.
Issues
The first step towards #1108.
Testing