feat: added support for multi_config #984
Conversation
WalkthroughThe recent changes involve updates to the handling of build directories in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java (2 hunks)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java (8 hunks)
- bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (17 hunks)
Files skipped from review due to trivial changes (1)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java
Additional comments not posted (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java (2)
182-191: Refactored extraction of custom build directory from CMake arguments.The refactoring from a direct string search to using an array and iterating through it is a more robust and clear method for handling the extraction of the
-Bargument. This change should help in avoiding potential bugs related to string manipulation and makes the code easier to understand and maintain.
194-194: Utilization of the newIDFUtil.setBuildDirmethod to set the build directory.This change centralizes the setting of the build directory into a utility method, which is a good practice as it abstracts the logic away from the client code, making it reusable and easier to maintain. Ensure that the
setBuildDirmethod handles all edge cases appropriately.bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (2)
12-12: Added imports to support new functionality.These imports are necessary for the new methods and functionalities introduced in this file.
Also applies to: 14-14, 18-19, 22-22, 72-72, 74-74
152-152: Enhanced build folder management during SDK configuration editing.The changes introduce a more robust handling of the build folder during the configuration editing process, including setting and rolling back the build folder based on the location of the sdkconfig. This is a good practice to ensure that the environment reflects the current state of the configuration being edited.
Also applies to: 153-153, 160-165, 186-189, 215-216
| String buildFolder = StringUtil.EMPTY; | ||
| try | ||
| { | ||
| buildFolder = IDFUtil.getBuildDir(project); | ||
| } | ||
| catch (CoreException e) | ||
| { | ||
| Logger.log(e); | ||
| } | ||
| return buildFolder; | ||
| } |
There was a problem hiding this comment.
Refactored methods for build folder management and SDK configuration location checks.
The refactoring of these methods improves the clarity and maintainability of the code. It's good to see that optional and exception handling are used effectively to manage the build directory settings and checks.
Also applies to: 1009-1019, 1021-1026, 1028-1036
sigmaaa
changed the title
|
@sigmaaa hi ! Tested under: able to build multi config to same project. ✔️ able to open every sdkconfig file ✔️
|
Hi @AndriiFilippov, @kolipakakondal, fixed in the last commit, PTAL |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bundles/com.espressif.idf.sdk.config.core/src/com/espressif/idf/sdk/config/core/server/ConfigServerManager.java (2 hunks)
Additional comments not posted (3)
bundles/com.espressif.idf.sdk.config.core/src/com/espressif/idf/sdk/config/core/server/ConfigServerManager.java (3)
52-52: LGTM! The addition ofthrows IOExceptionis appropriate.The
throws IOExceptionclause ensures that any IO exceptions encountered during the creation or starting of theJsonConfigServerare properly propagated.
75-76: LGTM! The updated comparison logic forfileattributes is more accurate.Using
getLocation()for comparison improves the accuracy of theequalsmethod.
82-82: LGTM! The updatedhashCodemethod ensures consistency withequals.Using
Objects.hashwithproject.getName()andfile.getLocation()ensures that the hash code is consistent with theequalsmethod.
|
@sigmaaa hi ! Tested under: LGTM 👍 There is some mistakes in docs, please update it. Example does not work with provided command sdkconfig.prod1sdkconfig.prod1 Same here and this typo as well: Moreover, I was not able to set different sdkconfigs for particular "build" folder, |
|
@sigmaaa hi ! Tested under: LGTM 👍 |
Description
added ability to run sdkconfig from any build folder
Fixes # (IEP-1250)
Type of change
Please delete options that are not relevant.
How has this been tested?
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Enhancements