-
Notifications
You must be signed in to change notification settings - Fork 317
New Samples project for building all samples #3740
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
base: main
Are you sure you want to change the base?
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 restructures the MSBuild directory hierarchy by moving the root Directory.Build.props from src/ to the repository root, establishing a centralized build configuration approach. This reorganization ensures consistent build properties across the entire repository including source code, tests, and documentation samples.
Key changes:
- Created a new root
Directory.Build.propsandDirectory.Packages.propsat repository level - Updated import paths in subdirectories to reference the new root-level build files
- Added a new samples project (
Microsoft.Data.SqlClient.Samples.csproj) to the solution for building documentation code samples - Updated namespace names in sample files for consistency
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Build.props | New root-level build properties file with all core MSBuild settings moved from src/ |
| Directory.Packages.props | Reformatted with removed blank lines and added Microsoft.Identity.Client package |
| src/Directory.Build.props | Simplified to import parent Directory.Build.props |
| src/Directory.Package.props | New file importing parent Directory.Packages.props |
| src/Microsoft.Data.SqlClient/tests/Directory.Packages.props | Updated import path to reference root-level file |
| src/Microsoft.Data.SqlClient/add-ons/Directory.Packages.props | Updated import path to reference root-level file |
| doc/Directory.Build.props | New file importing tests Directory.Build.props |
| doc/Directory.Package.props | New file importing root Directory.Packages.props |
| doc/samples/Microsoft.Data.SqlClient.Samples.csproj | New samples project for building documentation code samples |
| doc/samples/*.cs | Updated namespaces and added conditional compilation for framework-specific samples |
| src/Microsoft.Data.SqlClient.sln | Added samples project to solution |
| build.proj | Updated import path and added Samples item group |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3740 +/- ##
==========================================
- Coverage 76.83% 76.57% -0.26%
==========================================
Files 272 272
Lines 45363 44177 -1186
==========================================
- Hits 34854 33828 -1026
+ Misses 10509 10349 -160
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:
|
|
@cheenamalhotra Does the new project build? How do you solve the namespace challenges? This seems like a lot of product changes to add a validation build of documentation samples. in my POC, I had simply added a single .csproj in the samples folder (only challenge then was the namespaces) <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net8.0:net9.0;net472</TargetFrameworks>
<ImplicitUsings>disable</ImplicitUsings>
<Nullable>disable</Nullable>
<UseWindowsForms>true</UseWindowsForms>
<LangVersion>10.0</LangVersion>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Data.SqlClient" Version="7.0.0-preview2.25289.6" />
<PackageReference Include="Microsoft.SqlServer.Server" Version="1.0.0" />
</ItemGroup>
</Project>
|
Gonna revisit this, to make it simpler as well as do a public driver reference instead. This was a first hand experiment to understand impact of this work, which is indeed a lot (~700 errors). As of now the solution will not build the project so it will need to be worked upon to fix failures. I mainly want to enable that and allow community to participate in helping us clear out compilation errors, maintaining sample's expected design and behavior. |
762a382 to
70db11b
Compare
|
@cheenamalhotra Sounds good, once the project is in I have some ideas how to make it build. |
Addresses #3725 - this PR adds a new Sample project for build support.
There are issues with samples that need to be addressed gradually and can be done with community support, but this PR introduces a basic scaffolding to initiate build support.