Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @MingboPeng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the project's build infrastructure by enabling multi-targeting for both .NET Framework 4.8 and .NET 8 across key applications. It introduces a robust Makefile to automate the compilation and packaging of these multi-targeted builds, ensuring a more organized and efficient development workflow. The changes also refine how build outputs are structured and managed, improving overall project maintainability.
Highlights
- Multi-Targeting Support: The
Ironbug.GrasshopperandIronbug.Consoleprojects have been updated to multi-target both .NET Framework 4.8 and .NET 8, allowing for broader compatibility and leveraging newer framework features. - Automated Build Process: A new
Makefilehas been introduced to streamline the build process, including commands for compiling, packaging, and organizing outputs for different target frameworks and platforms (Windows and Linux). - Improved Build Output Management: Project files (
.csproj) now append the target framework to their output paths, ensuring that build artifacts for different frameworks are properly separated and managed, which is crucial for multi-targeting. - Git Ignore Updates: The
.gitignorefile has been updated to exclude new build output directories (installer/plugin/andinstaller/HVACTemplates/), preventing temporary or generated files from being committed to the repository.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates the project to support multi-targeting for .NET 8 and .NET Framework 4.8. The changes in the .csproj files are well-implemented for this purpose. A new Makefile is introduced to orchestrate the build process. My review focuses on improving the robustness and maintainability of the new Makefile. I've identified a couple of issues that could cause the build to fail and some inconsistencies that could be improved for better clarity.
| dotnet build ./src/Ironbug.Grasshopper/Ironbug.Grasshopper.csproj /p:Configuration=Release /p:Platform=x64 /p:Version=$(NEW_RELEASE_VERSION) /restore | ||
| mkdir -p installer/plugin | ||
| mv src/Ironbug.Grasshopper/bin/x64/Release/net8-windows/* installer/plugin/ | ||
| mv src/Ironbug.Grasshopper/bin/x64/Release/net48/* installer/plugin/net48/ |
There was a problem hiding this comment.
| rm ./src/Ironbug.Console/bin/x64/Release/net8/openstudio* -r | ||
|
|
||
| cp ./src/Ironbug.Console/bin/x64/Release/net8/* installer/plugin | ||
| cp ./src/Ironbug.Console/bin/x64/Release/net48/* installer/plugin/net48/ |
There was a problem hiding this comment.
The cp command on this line has an implicit dependency on the build-Grasshopper target to create the installer/plugin/net48/ directory. This makes the build process fragile. For instance, running this target alone would fail. To make this target more robust and self-contained, create the directory before copying files into it.
mkdir -p installer/plugin/net48
cp ./src/Ironbug.Console/bin/x64/Release/net48/* installer/plugin/net48/
| rm ./src/Ironbug.Console/bin/x64/Release/net48/openstudio* -r | ||
| rm ./src/Ironbug.Console/bin/x64/Release/net8/openstudio* -r |
There was a problem hiding this comment.
While placing options after file arguments is valid for GNU rm, it's unconventional and can be less readable. For better clarity and to adhere to common practice, it's recommended to place options before the file arguments.
rm -r ./src/Ironbug.Console/bin/x64/Release/net48/openstudio*
rm -r ./src/Ironbug.Console/bin/x64/Release/net8/openstudio*
| rm -r installer/plugin/net48 | ||
|
|
||
| build-console-linux: | ||
| dotnet build ./src/Ironbug.Console/Ironbug.Console.csproj -a x64 /p:Configuration=Release /p:TargetFramework=net8 /p:Version=$(NEW_RELEASE_VERSION) |
There was a problem hiding this comment.
This dotnet build command is inconsistent with the other build commands in this file. For better maintainability and predictability, it's good to keep them consistent.
Specifically:
- It uses
-a x64while others use/p:Platform=x64. - It's missing the
/restoreflag, which is explicitly used in other commands. Whiledotnet buildperforms an implicit restore, being explicit improves clarity and consistency.
dotnet build ./src/Ironbug.Console/Ironbug.Console.csproj /p:Platform=x64 /p:Configuration=Release /p:TargetFramework=net8 /p:Version=$(NEW_RELEASE_VERSION) /restore
|
🎉 This PR is included in version 1.19.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.