#317: added BuildCommandlet #340
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added BuildCommandlet added BuildCommandletTest added BuildCommandletTest resources
jan-vcapgemini
added
enhancement
set npm test to parameterized to enforce windows only
# Conflicts: # cli/src/main/resources/nls/Ide.properties # cli/src/main/resources/nls/Ide_de.properties
added missing val path property to nls files adjusted assertLogMessage (removed parameterized test)
Pull Request Test Coverage Report for Build 9806378442Details
💛 - Coveralls |
aBega2000
approved these changes
May 23, 2024
hohwille
reviewed
May 24, 2024
cli/src/main/java/com/devonfw/tools/ide/commandlet/BuildCommandlet.java
Outdated
Show resolved
Hide resolved
hohwille
reviewed
May 24, 2024
cli/src/main/java/com/devonfw/tools/ide/commandlet/BuildCommandlet.java
Outdated
Show resolved
Hide resolved
hohwille
requested changes
May 24, 2024
There was a problem hiding this comment.
@jan-vcapgemini thanks for this PR. You did a great job implementing this and especially testing this commandlet with all its scenarios. 👍
However, there is some concern with the CLI properties and related behavior that differs from devonfw-ide. Please have a look at my review comments.
changed PathProperty to multivalued StringPropertery adjusted tests
added build documentation added mvn documentation added npm documentation added yarn documentation
replaced devon with ide
added default build options environment variables and handling added gradle documentation added default options to documentations added test for default options
hohwille
approved these changes
Jun 13, 2024
There was a problem hiding this comment.
@jan-vcapgemini Thanks for your update. You did some great improvements 👍
Probably for complex stories I should next time do the reverse-engineering myself and write everything explicitly as requirements and acceptance criteria into the story. Due to lack of time, I created all the commandlet stories via copy&paste with a simple link to the old code and expected the developer to do the reverse-engineering. As it turns out this seems to be more tricky than expected...
Please have a look at my review comments.
cli/src/main/java/com/devonfw/tools/ide/commandlet/BuildCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/commandlet/BuildCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/variable/IdeVariables.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/variable/IdeVariables.java
Outdated
Show resolved
Hide resolved
cli/src/test/resources/ide-projects/build/project/settings/ide.properties
Outdated
Show resolved
Hide resolved
hohwille
reviewed
Jun 13, 2024
# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/variable/IdeVariables.java # cli/src/main/resources/nls/Help.properties # cli/src/main/resources/nls/Help_de.properties
…dlet.java Co-authored-by: Jörg Hohwiller <[email protected]>
added default 'clean install' to mvn build opts added default 'clean dist' to gradle build opts
# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/variable/IdeVariables.java # cli/src/main/resources/nls/Help.properties # cli/src/main/resources/nls/Help_de.properties
removed help adoc files added build.detail to help properties added German build.detail to help_de properties
adjusted default build params for mvn in tests and resources
added TODO with link to issue
adjusted order to fix test
added exception if build path is null
hohwille
approved these changes
Jul 5, 2024
There was a problem hiding this comment.
@jan-vcapgemini thanks for your updates. Now you got everything right 👍
Ready for merge.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #317
Implements