#809: Enhance uninstall with --force to remove from the software repo#1242
Conversation
… software repo Implements: functions to fully delete a tool from the _ide software repository
Pull Request Test Coverage Report for Build 15062852418Details
💛 - Coveralls |
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
jan-vcapgemini
left a comment
There was a problem hiding this comment.
@juliane-cap thanks for the enhancement of the uninstall commandlet with the force mode.
Everything looks fine to me. It would be nice if you could also add some simple tests which ensure that the software was actually deleted from the repository.
I've also added some small change requests, please check these.
|
@jan-vcapgemini would LocalToolCommandlet.forceUninstall() work on macOs or should i use the macOsHelper to get the real path? |
Add macOS workaround to find the right path
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
mention --force uninstall in Help
jan-vcapgemini
left a comment
There was a problem hiding this comment.
Thanks for adding a new test and implementing the CRs. I've added some more very small CRs, after these are solved, the PR is ready for review.
cli/src/test/java/com/devonfw/tools/ide/commandlet/UninstallCommandletTest.java
Outdated
Show resolved
Hide resolved
cli/src/test/java/com/devonfw/tools/ide/commandlet/UninstallCommandletTest.java
Outdated
Show resolved
Hide resolved
cli/src/test/java/com/devonfw/tools/ide/commandlet/UninstallCommandletTest.java
Outdated
Show resolved
Hide resolved
Your current implementation with the added check for MacOs should work fine I think. |
Refactor path format in UninstallCommandletTest, Double-check for macOS before applying path change in LocalToolCommandlet
add function to get the right path incase the path points to the wrong path with macOS
hohwille
left a comment
There was a problem hiding this comment.
@juliane-cap thanks for your PR and rework. Great job and nice improvements (including help texts, etc.). 👍
I still left some review comments for improvement. When resolved we can finally merge.
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java
Outdated
Show resolved
Hide resolved
Fix method getValidInstalledSoftwareRepoPath to also work for special software with different repository naming styles
hohwille
left a comment
There was a problem hiding this comment.
@juliane-cap thanks for addressing all review comments. Perfect now and ready for merge Good job👍
cli/src/test/java/com/devonfw/tools/ide/commandlet/UninstallCommandletTest.java
Outdated
Show resolved
Hide resolved
…mmandletTest.java
|
I've tested these changes on Linux, Mac and Windows and it worked on all systems. |
Fixes: #809
Implements: