-
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-922 Original Launch Configuration disappears after creating the new one. #851
Conversation
WalkthroughThe Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- bundles/com.espressif.idf.launch.serial.core/src/com/espressif/idf/launch/serial/core/IDFCoreLaunchConfigProvider.java (5 hunks)
IProject project = descriptor.getAdapter(IProject.class); | ||
if (project != null) | ||
{ | ||
Map<String, ILaunchConfiguration> projectConfigs = configs.get(project); | ||
if (projectConfigs == null) | ||
{ | ||
projectConfigs = new HashMap<>(); | ||
configs.put(project, projectConfigs); | ||
} | ||
|
||
String targetConfig = descriptor.getName(); | ||
configuration = projectConfigs.get(targetConfig); | ||
configuration = configs.computeIfAbsent(project, key -> new HashMap<>()).get(targetConfig); | ||
if (configuration == null) | ||
{ | ||
// do we already have one with the descriptor? |
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.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [27-42]
The computeIfAbsent
method is used to simplify the initialization and retrieval of launch configurations. This is a good use of Java 8's Map
API and improves the readability of the code.
} | ||
if (configuration.exists()) | ||
{ | ||
Map<String, ILaunchConfiguration> projectConfigs = configs.get(project); | ||
if (projectConfigs == null) | ||
{ | ||
projectConfigs = new HashMap<>(); | ||
configs.put(project, projectConfigs); | ||
} | ||
|
||
projectConfigs.put(configuration.getName(), configuration); | ||
configs.computeIfAbsent(project, key -> new HashMap<>()).put(configuration.getName(), configuration); | ||
} |
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.
The computeIfAbsent
method is used to simplify the initialization and storage of launch configurations. This is a good use of Java 8's Map
API and improves the readability of the code.
@sigmaaa hi! Tested under: LGTM 👍 |
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.
self reviewed
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
Looks like
launchBarManager.launchObjectRemoved(projectEntry.getKey());
is not necessary to call fromlaunchConfigurationRemoved
method. It was added in IEP-442 PR. I've tested the mentioned test cases there and looks fine without these changes, so I removed this override method.Also, I've made small refactoring by using computeIfAbsence to make other methods more clear
Fixes # (IEP-922)
Type of change
Please delete options that are not relevant.
How has this been tested?
Test 1:
Test 2:
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit