#1880: Allow reset of installed plugins when launching IDE in force mode#1891
#1880: Allow reset of installed plugins when launching IDE in force mode#1891areinicke wants to merge 10 commits intodevonfw:mainfrom
Conversation
Coverage Report for CI Build 25566036672Coverage decreased (-0.04%) to 70.645%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions13 previously-covered lines in 2 files lost coverage.
Coverage Stats💛 - Coveralls |
|
I tested the -f option with vscode and intellij. With vscode, everything works and looks good! With intellij i get this one always (not sure if this comes from your changes, i guess not...): Also happens without the -f option, so i guess this is another error...your changes look good! |
|
Yeah. The PlantUML extension seems to be broken. But like you said, this has nothing to do with my changes. |
hohwille
left a comment
There was a problem hiding this comment.
@areinicke thank you for your story idea and this PR implementing it. Great that you also found the perfect spot to document the new feature. 👍
I do have one concern and would like to suggest some improvement via my review comment. Please have a look and consider a small upgrade of this PR. Thanks.
| boolean resetPlugins = this.context.question( | ||
| "You are launching " + getName() + " in force mode. Do you want to reset all plugins for " + getName() + "? " | ||
| + "This will uninstall all currently installed plugins and reinstall them as configured in your IDEasy project settings."); | ||
| if (resetPlugins) { | ||
| deleteAllPlugins(pluginsInstallationPath); | ||
| } |
There was a problem hiding this comment.
We should limit questions only to the bare minimum (dangerous situations) for UX reasons.
If you want to avoid accidents you can add an explicit FlagProperty called --force-plugins or --force-plugin-reinstall. Example:
I would actually recommend that approach. See also issue #1007
But if you run this use-case frequently you will IMHO start getting annoyed by such question.
If the option --force will remove and reinstall all plugins, then it should do that without asking.
If I do not want to have it, then I do not use --force option.
| boolean resetPlugins = this.context.question( | |
| "You are launching " + getName() + " in force mode. Do you want to reset all plugins for " + getName() + "? " | |
| + "This will uninstall all currently installed plugins and reinstall them as configured in your IDEasy project settings."); | |
| if (resetPlugins) { | |
| deleteAllPlugins(pluginsInstallationPath); | |
| } | |
| deleteAllPlugins(pluginsInstallationPath); |
| SystemInfo systemInfo = SystemInfoMock.of(os); | ||
| IdeTestContext context = newContext(PROJECT_ECLIPSE, "eclipseproject"); | ||
| context.setSystemInfo(systemInfo); | ||
| context.getStartContext().setForceMode(true); // #663 |
There was a problem hiding this comment.
This already confirms my suggestion that we should use an explicit flag.
Seems the test was broken by the change since force mode would now cause additional side-effects breaking the existing test.
This PR fixes #1880
Implemented changes:
Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal