-
Notifications
You must be signed in to change notification settings - Fork 403
🔨Split build scripts for NanoFramework #1627
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
Conversation
This refactoring improves the build system's cross-platform compatibility by separating Windows-only NanoFramework builds from the main dotnet CLI builds. Changes: - Split Start-Build into Start-Build (dotnet CLI) and Start-BuildNanoFramework (MSBuild) - Split Start-PackNugets into Start-PackNugets (dotnet pack) and Start-PackNugetsNanoFramework (NuGet.exe) - Made Start-Build and Start-PackNugets use only dotnet CLI for cross-platform support - Updated build.ps1 to conditionally call NanoFramework functions with -IncludeNanoFramework flag - Added Visual Studio/MSBuild prerequisite checks to NanoFramework functions - Export new functions from build-functions module This allows the main build to work on any platform with .NET SDK installed, while NanoFramework builds remain optional and Windows-only. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Code ReviewSummaryThis refactoring improves cross-platform build support by cleanly separating Windows-specific NanoFramework builds from the main dotnet CLI builds. The changes are well-structured and maintain backward compatibility. ✅ Positive Changes
📝 Observations & Suggestions1. Local settings file includedThe PR includes 2. Function export cleanupGood catch removing the unused 3. Enhanced error messagesThe new error messages clearly guide users when prerequisites are missing (Visual Studio/MSBuild for NanoFramework builds), which improves developer experience. ✅ No Breaking Changes Detected
✅ Code Quality
Performance ConsiderationsNo performance concerns - the refactoring maintains the same build operations, just reorganizes them for better maintainability. Test CoverageThe changes are to build scripts which are validated through CI/CD pipeline execution. No additional test coverage needed. SecurityNo security concerns identified. ConclusionThis is a well-executed refactoring that improves the build system's maintainability and cross-platform compatibility without introducing breaking changes. The only minor suggestion is to review whether the Recommendation: Approve ✅ |
|
This should be completed with #1625 as it is right now not publishing all packages. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1627 +/- ##
======================================
Coverage 96% 96%
======================================
Files 322 322
Lines 27531 27531
======================================
Hits 26488 26488
Misses 1043 1043 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This refactoring improves the build system's cross-platform compatibility by separating Windows-only NanoFramework builds from the main dotnet CLI builds.
Changes:
This allows the main build to work on any platform with .NET SDK installed, while NanoFramework builds remain optional and Windows-only.
🤖 Generated with Claude Code