#142 and #94: Plugin based commandlet feature and PluginBasedCommandlet #153

diiinesh · 2023-12-11T12:10:48Z

fixes #142:

created an abstract class "PluginBasedCommandlet" with all the plugin releated functions

fixes #94

modified getPluginsMap function, such that plugins can be processed with higher priority in user home directory

…nction, such that user can configure plugins in their home directory as well

cli/src/main/java/com/devonfw/tools/ide/tool/ide/IdeToolCommandlet.java

cli/src/main/java/com/devonfw/tools/ide/tool/PluginBasedCommandlet.java

hohwille

@diiinesh I am aware this is still in draft. However, due to a question I was asked to answer in this PR I already looked at your code.
You did a good job and are very close to the final result. 👍
IMHO you can address my review comments and then take this PR out of draft mode and move it to the review so it can get merged.

…uginBasedCommandlet, also changed method of getting home directory

coveralls · 2023-12-15T13:08:43Z

Pull Request Test Coverage Report for Build 7222434854

0 of 0 changed or added relevant lines in 0 files are covered.
32 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.02%) to 47.856%

Files with Coverage Reduction	New Missed Lines	%
com/devonfw/tools/ide/tool/ide/IdeToolCommandlet.java	32	6.49%

Totals
Change from base Build 7220702365:	-0.02%
Covered Lines:	2719
Relevant Lines:	5455

💛 - Coveralls

…and uninstallPlugin, changed PluginBasedCommandlet.java to abstract class

hohwille

@diiinesh thanks for your PR.
It seems that git did play some trick on you and something went wrong in this PR reverting lots of changes/improvements that others have done by accident.
Please have a look. See also PR #141

hohwille · 2024-01-18T17:28:58Z

cli/src/main/java/com/devonfw/tools/ide/tool/PluginBasedCommandlet.java

+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.*;


your IDE settings are outdated/incorrect. Star-Imports are discouraged. According to our IDE settings this should also be auto-configured correctly for Intellij (maybe ask @jan-vcapgemini if it is not working for you) and for Eclipse it was correct from the beginning.

hohwille · 2024-01-18T17:31:58Z

cli/src/test/java/com/devonfw/tools/ide/commandlet/EnvironmentCommandletTest.java

-  /**
-   * Test of {@link EnvironmentCommandlet} run.
-   */
-  @Test
-  public void testRun() {
-
-    // arrange
-    String path = "workspaces/foo-test/my-git-repo";
-    IdeTestContext context = newContext("basic", path, false);
-    EnvironmentCommandlet env = context.getCommandletManager().getCommandlet(EnvironmentCommandlet.class);
-    // act
-    env.run();
-    // assert
-    assertLogMessage(context, IdeLogLevel.INFO, "MVN_VERSION=3.9.*");
-    assertLogMessage(context, IdeLogLevel.INFO, "SOME=some-${UNDEFINED}");
-    assertLogMessage(context, IdeLogLevel.INFO, "BAR=bar-some-${UNDEFINED}");
-    assertLogMessage(context, IdeLogLevel.INFO, "IDE_TOOLS=mvn,eclipse");
-    assertLogMessage(context, IdeLogLevel.INFO, "ECLIPSE_VERSION=2023-03");
-    assertLogMessage(context, IdeLogLevel.INFO, "FOO=foo-bar-some-${UNDEFINED}");
-    assertLogMessage(context, IdeLogLevel.INFO, "JAVA_VERSION=17*");
-    assertLogMessage(context, IdeLogLevel.INFO, "INTELLIJ_EDITION=ultimate");
-    assertLogMessage(context, IdeLogLevel.INFO, "DOCKER_EDITION=docker");
-  }
-  /**
-   * Test of {@link EnvironmentCommandlet} does not require home.
-   */


your commit is removing and reverting a lot of things that seems to be undesired.
I already reverted some things in the browser but that is rather tedious.
IMHO you did something wrong with git here to lose/revert changes that had made by others as improvements.
See also entire removal of ConextCommandletTest.java what is IMHO a mistake.
I already reverted the removal of the coverall stuff added by Moritz.

hohwille · 2024-01-18T17:33:32Z

cli/src/test/java/com/devonfw/tools/ide/tool/AbstractPluginBasedCommandletTest.java

+  @Test
+  public void testGetPluginsMap() {
+
+
+
+
+
+  }


does not seem to be complete.
Why is a @test annotation inside a Commandlet? This does not make sense.
Either this is a test so it may extend Assertions or other test-classes or it is a commandlet but not a test.

hohwille · 2024-01-18T17:35:24Z

pom.xml

-      <plugin>
-        <groupId>org.jacoco</groupId>
-        <artifactId>jacoco-maven-plugin</artifactId>
-        <version>0.8.11</version>
-        <executions>
-          <execution>
-            <goals>
-              <goal>prepare-agent</goal>
-            </goals>
-          </execution>
-          <execution>
-            <id>report</id>
-            <phase>prepare-package</phase>
-            <goals>
-              <goal>report</goal>
-            </goals>
-          </execution>
-        </executions>
-      </plugin>


was added by Moritz on purpose. Do not remove.

hohwille · 2024-01-18T17:36:42Z

cli/src/test/java/com/devonfw/tools/ide/commandlet/HelpCommandletTest.java

-    assertThat(help.isIdeHomeRequired()).isFalse();
+    assertThat(help.isIdeHomeRequired()).isEqualTo(false);


This was also an improvement that you are reverting.

hohwille · 2024-01-18T17:37:14Z

README.adoc

@@ -10,7 +10,6 @@ image:https://img.shields.io/github/license/devonfw/IDEasy.svg?label=License["Ap
 image:https://img.shields.io/maven-central/v/com.devonfw.tools.ide/ide-cli.svg?label=Maven%20Central["Maven Central",link=https://search.maven.org/search?q=g:com.devonfw.tools.ide]
 image:https://github.com/devonfw/IDEasy/actions/workflows/build.yml/badge.svg["Build Status",link="https://github.com/devonfw/IDEasy/actions/workflows/build.yml"]
 image:https://github.com/devonfw/IDEasy/actions/workflows/update-urls.yml/badge.svg["Update URLS Status",link="https://github.com/devonfw/IDEasy/actions/workflows/update-urls.yml"]
-image:https://coveralls.io/repos/github/devonfw/IDEasy/badge.svg?branch=main["Coverage Status",link="https://coveralls.io/github/devonfw/IDEasy?branch=main"]


please keep this badge added by Moritz for the coverage.

hohwille · 2024-01-18T17:51:54Z

To avoid double effort, I will close this PR and we concentrate on PR #141 that includes the changes of this PR and already fixed some of the problems.

diiinesh added 2 commits December 8, 2023 12:07

added abstract class PluginBasedCommandlet, enhanced getPluginsMap fu…

77d6b69

…nction, such that user can configure plugins in their home directory as well

added abstract class PluginBasedCommandlet, enhanced getPluginsMap fu…

8cf6ec4

…nction, such that user can configure plugins in their home directory as well

diiinesh changed the title ~~Plugin based commandlet feature~~ #142 and #94: Plugin based commandlet feature and PluginBasedCommandlet Dec 11, 2023

hohwille reviewed Dec 14, 2023

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/tool/ide/IdeToolCommandlet.java Outdated Show resolved Hide resolved

hohwille reviewed Dec 14, 2023

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/tool/PluginBasedCommandlet.java Outdated Show resolved Hide resolved

hohwille reviewed Dec 14, 2023

View reviewed changes

cli/src/main/java/com/devonfw/tools/ide/tool/PluginBasedCommandlet.java Outdated Show resolved Hide resolved

hohwille reviewed Dec 14, 2023

View reviewed changes

hohwille mentioned this pull request Dec 14, 2023

#93: Support for Maven Plugins #141

Merged

diiinesh added 2 commits December 15, 2023 13:26

removed functions in IdeToolCommandlet which were already moved to Pl…

e7e1c18

…uginBasedCommandlet, also changed method of getting home directory

removed unused imports

dd48f51

diiinesh marked this pull request as ready for review December 15, 2023 13:06

Merge branch 'main' into PluginBasedCommandlet_Feature

70b0424

diiinesh self-assigned this Dec 15, 2023

diiinesh added 3 commits January 15, 2024 14:50

wrote test for getPluginsMap function, moved function: installPlugin …

75bce3b

…and uninstallPlugin, changed PluginBasedCommandlet.java to abstract class

removed unused imports

f84612b

test plugin files for eclipse

c6fd6bb

diiinesh assigned hohwille and unassigned diiinesh Jan 15, 2024

hohwille added 4 commits January 18, 2024 18:16

Merge branch 'main' into PluginBasedCommandlet_Feature

9f26c0a

revert undesired change to remove coveralls

aa80a36

revert undesired removal of coveralls

0eebba1

trying to resolve compile/merge errors in browser what is tedious

0006965

hohwille requested changes Jan 18, 2024

View reviewed changes

hohwille closed this Jan 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#142 and #94: Plugin based commandlet feature and PluginBasedCommandlet #153

#142 and #94: Plugin based commandlet feature and PluginBasedCommandlet #153

diiinesh commented Dec 11, 2023 •

edited

Loading

hohwille left a comment •

edited

Loading

coveralls commented Dec 15, 2023

hohwille left a comment

hohwille Jan 18, 2024

hohwille Jan 18, 2024

hohwille Jan 18, 2024

hohwille Jan 18, 2024

hohwille Jan 18, 2024

hohwille Jan 18, 2024

hohwille commented Jan 18, 2024

		assertThat(help.isIdeHomeRequired()).isFalse();
		assertThat(help.isIdeHomeRequired()).isEqualTo(false);

#142 and #94: Plugin based commandlet feature and PluginBasedCommandlet #153

#142 and #94: Plugin based commandlet feature and PluginBasedCommandlet #153

Conversation

diiinesh commented Dec 11, 2023 • edited Loading

hohwille left a comment • edited Loading

Choose a reason for hiding this comment

coveralls commented Dec 15, 2023

Pull Request Test Coverage Report for Build 7222434854

💛 - Coveralls

hohwille left a comment

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille Jan 18, 2024

Choose a reason for hiding this comment

hohwille commented Jan 18, 2024

diiinesh commented Dec 11, 2023 •

edited

Loading

hohwille left a comment •

edited

Loading