-
Notifications
You must be signed in to change notification settings - Fork 121
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
IEP-1198 Unclear message when another ESP-IDF path is found #932
Conversation
WalkthroughThis update introduces clearer user instructions and options for handling different ESP-IDF paths, alongside code refactoring for better readability and maintenance. It enhances the user experience by providing explicit choices when a discrepancy in ESP-IDF paths is detected, ensuring a smoother setup process. Changes
Possibly related issues
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 (
|
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/Messages.java (1 hunks)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/messages.properties (1 hunks)
- bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/InitializeToolsStartup.java (5 hunks)
Additional comments not posted (9)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/Messages.java (2)
32-32
: The addition ofToolsInitializationDifferentPathMessageBoxOptionYes
enhances clarity in user options. Good job.
33-33
: The addition ofToolsInitializationDifferentPathMessageBoxOptionNo
is a clear way to present options to the user. Well done.bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/messages.properties (3)
22-22
: Updating the message to specify the configuration file and provide clear instructions improves user understanding. Nicely done.
23-23
: Adding a clear affirmative option (Use New Path
) for the user is a thoughtful touch.
24-24
: Adding a clear negative option (Use Old Path
) for the user enhances decision-making. Good addition.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/InitializeToolsStartup.java (4)
15-15
: The addition of thejava.util.Map
import supports the use ofMap.of
for setting button labels. This is a necessary change.
119-120
: UsingMap.of
for setting button labels is a clean and modern approach. This enhances code readability and maintainability.
128-129
: The adjustments in method call parameters forupdateEspIdfJsonFile
are correctly aligned with the changes made in the message box options. This ensures consistency in user choices.
119-129
: The code formatting changes, including the use ofMap.of
and adjustments in method call parameters, contribute positively to the code's readability and maintainability.
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.
LGTM
Description
Make warning message when different IDF version is detected in esp-idf.json more intuitive
Fixes # (IEP-1198)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit