Adopt build-standards for shared .NET configuration#1023
Merged
Conversation
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on f795c1b..c6a9fbc
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (8)
• .github/workflows/test-publish.yml
• .gitmodules
• Directory.Build.props
• Directory.Packages.props
• nccid.Test/nccid.Test.csproj
• nccid.Tests/nccid.Tests.csproj
• nccid.sln
• nccid/nccid.csproj
⏭️ Files skipped (5)
| Locations |
|---|
nccid.Tests/CsvMergerTests.cs |
nccid.Tests/DataTests.cs |
nccid.Tests/OptionsTests.cs |
nccid.Tests/UnitTest1.cs |
nccid.Tests/UtilsTests.cs |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
jas88
force-pushed
the
adopt-build-standards
branch
5 times, most recently
from
7ec5c99 to
2bd4468
Compare
- Add build-standards submodule for shared build properties - Enable central package management via Directory.Packages.props - Simplify csproj files (versions now centralized) - Rename nccid.Test → nccid.Tests to match convention - Replace .editorconfig with symlink to build-standards - Disable AOT/trimming (uses reflection-based JSON serialization)
jas88
force-pushed
the
adopt-build-standards
branch
from
2bd4468 to
b25af1b
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adopt the shared
build-standardssubmodule for consistent .NET build configuration across jas88 projects.Changes
Directory.Packages.propsnccid.Test→nccid.Teststo match conventionBenefits
High-level PR Summary
This PR adopts a shared
build-standardssubmodule to standardize .NET build configuration across the project. It introduces central package management viaDirectory.Packages.props, simplifies project files by inheriting common build properties from the submodule, renames the test project fromnccid.Testtonccid.Teststo follow naming conventions, and replaces the local.editorconfigwith a reference to a shared configuration. The changes also explicitly disable AOT and trimming in the main project due to its use of reflection-based JSON serialization.⏱️ Estimated Review Time: 30-90 minutes
💡 Review Order Suggestion
.gitmodulesbuild-standardsDirectory.Build.propsDirectory.Packages.props.editorconfignccid/nccid.csprojnccid.Tests/nccid.Tests.csprojnccid.sln.github/workflows/test-publish.ymlnccid.Tests/CsvMergerTests.csnccid.Tests/DataTests.csnccid.Tests/OptionsTests.csnccid.Tests/UnitTest1.csnccid.Tests/UtilsTests.csSummary by cubic
Standardizes .NET build config by adopting the shared build-standards submodule and moving all package versions to central management. Simplifies project files and renames the test project to nccid.Tests.
Refactors
Migration
Written for commit b25af1b. Summary will update on new commits.