Skip to content

#1953: Implement base functionality of Cleanup Commandlet - #1957

Open
areinicke wants to merge 27 commits into
devonfw:mainfrom
areinicke:feature/1580-cleanup-commandlet
Open

#1953: Implement base functionality of Cleanup Commandlet#1957
areinicke wants to merge 27 commits into
devonfw:mainfrom
areinicke:feature/1580-cleanup-commandlet

Conversation

@areinicke

@areinicke areinicke commented May 20, 2026

Copy link
Copy Markdown
Contributor

This PR fixes #1953

Implemented changes:

  • Implemented the base functionality of the cleanup commandlet. Users can run ide cleanup to scan for unused tool installations and automatically remove them. A tool is considered as unused if no project links to the tool installation from its software folder at $IDE_ROOT/<project>/software or $IDE_ROOT/<project>/software/extra.
  • Users will be given a summary report and are prompted before any deletion takes place. The confirmation prompt can be skipped by providing the flag --fd.
  • Added ide help documentation for the new commandlet
  • Added a basic test for the new commandlet. This will need to be expanded in th future though.

Testing instructions

Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:

  1. [Optional] Install some additional software using IDEasy. You can install additional software for IDEasy projects by switching around the required editions and versions of a tool using the set-edition and set-version commandlet repeatedly.
  2. Run ide cleanup
  3. Verify that the unused tool installations have been removed.

AND

  1. Run ide help and ide help cleanup to verify that the help documentation works as expected.

Checklist for this PR

Make sure everything is checked before merging this PR. For further info please also see
our DoD.

  • When running mvn clean test locally all tests pass and build is successful
  • PR title is of the form #«issue-id»: «brief summary» (e.g. #921: fixed setup.bat). If no issue ID exists, title only.
  • PR top-level comment summarizes what has been done and contains link to addressed issue(s)
  • PR and issue(s) have suitable labels
  • Issue is set to In Progress and assigned to you or there is no issue (might happen for very small PRs)
  • You followed all coding conventions
  • You have added the issue implemented by your PR in CHANGELOG.adoc unless issue is labeled
    with internal
  • You have formulated clear instructions on how to test your contribution under "Testing instructions"

@github-project-automation github-project-automation Bot moved this to 🆕 New in IDEasy board May 20, 2026
@areinicke areinicke self-assigned this May 20, 2026
@areinicke areinicke added enhancement New feature or request commandlet ide sub-command labels May 20, 2026
@areinicke areinicke moved this from 🆕 New to 🏗 In progress in IDEasy board May 20, 2026
@coveralls

coveralls commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 28829571525

Coverage decreased (-0.05%) to 72.013%

Details

  • Coverage decreased (-0.05%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 7 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

7 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
com/devonfw/tools/ide/commandlet/CommandletManagerImpl.java 7 91.75%

Coverage Stats

Coverage Status
Relevant Lines: 16762
Covered Lines: 12580
Line Coverage: 75.05%
Relevant Branches: 7510
Covered Branches: 4899
Branch Coverage: 65.23%
Branches in Coverage %: Yes
Coverage Strength: 3.18 hits per line

💛 - Coveralls

@areinicke
areinicke marked this pull request as ready for review May 21, 2026 06:54
@areinicke areinicke moved this from 🏗 In progress to Team Review in IDEasy board May 21, 2026

@Caylipp Caylipp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the cleanup commandlet locally.

For my manual test, I installed additional versions of gh and java and then ran ide cleanup. Only the extra unused versions were deleted, while the versions still referenced by a project were kept. So the basic deletion functionality worked for me.

I noticed an issue in ide shell where duplicate entries appeared on a second run. However, this issue has already been fixed.

Overall, the implementation looks good to me and my manual test worked. Very good!

@hohwille hohwille left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@areinicke thanks for your PR. Great that you divided the very large story and created a very reasonable starting point as first implementation that totally makes sense to me.
Also the logging as preview what will be deleted and the user confirmation looks good to me. Great job 👍
I left quite some review comments due to some conventions and design issues.
I hope you consider it as appreciation to learn some things to further advance your Java coding style and not as nasty picky pointless extra rework ;)

Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
private static class IdeTool {
String toolName;
private final Path path;
private final List<String> usedBy = new java.util.ArrayList<>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am slightly confused why we have this usedBy in all 3 inner classes.
If we really want to do that, we could even use OOP and derive the 3 classes from an abstract class with a getUsedBy() and a nice centralised JavaDoc that is then inherited and reused by all 3 classes.
But IMHO we only need to track what tool+edition+version combination is used.
If all unused ones are deleted and we end up with en empty edition folder this does not really waste disc-space and we can still check that after deletion we also delete the parent folder if it became empty (and if we delete the edition folder we can also delete the version folder if that is now also empty).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: Generic types like String or Number always need explanation. So if you see List<String> you do not know what this is unless you have further JavaDoc or other hints.
If you instead see List<IdeProject> and IdeProject is a class or record with JavaDoc then things immediately get clear. Such IdeProject type could wrap the project name as String but also maybe contain the Path to the project and later could even have methods like getWorkspaces() or whatever - see what @laim2003 is currently implementing for the GUI.

Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated
Comment thread cli/src/main/java/com/devonfw/tools/ide/commandlet/CleanupCommandlet.java Outdated

// assert
assertThat(context.getIdeRoot().resolve("_ide/software/default/az")).doesNotExist();
assertThat(context).logAtSuccess().hasMessage("Unused tools have been deleted successfully.");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice to have: you are just asserting that we logged this message and have deleted something/anything.
We should rather assert that something that is still used was not deleted and something that was unused has been deleted. Otherwise your implementation can be entirely broken but the test may still be green.

@github-project-automation github-project-automation Bot moved this from Team Review to 👀 In review in IDEasy board May 29, 2026
@hohwille hohwille assigned hohwille and areinicke and unassigned areinicke Jun 18, 2026

@hohwille hohwille left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@areinicke thanks for the update. Nice. 👍
Sorry, that I have one last concern following general Java conventions:
https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html

@Caylipp

Caylipp commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

The remaining review feedback has been addressed and all checks are passing. The PR already has an approving review and is ready to merge from my side.

@hohwille hohwille removed their assignment Jul 21, 2026
@Caylipp Caylipp moved this from 👀 In review to 🏗 In progress in IDEasy board Jul 27, 2026
@Caylipp

Caylipp commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

I completed the handover and addressed the remaining review feedback in #2230. This PR can be closed once #2230 has been reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commandlet ide sub-command enhancement New feature or request

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

Implement Base Functionality of Cleanup Commandlet

5 participants