Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#342: implement unit tests for mvn ToolCommandlet #373

Merged
merged 23 commits into from
Jul 4, 2024
Merged

#342: implement unit tests for mvn ToolCommandlet #373

merged 23 commits into from
Jul 4, 2024

Conversation

mvomiero
Copy link
Contributor

Closes: #342

@mvomiero mvomiero self-assigned this May 31, 2024
@mvomiero mvomiero added the mvn related to apache maven build tool label May 31, 2024
@coveralls
Copy link
Collaborator

coveralls commented Jun 3, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9797375875

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 12 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.4%) to 61.034%
Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/tool/mvn/Mvn.java 12 73.08%
Totals Coverage Status
Change from base Build 9778043214: 0.4%
Covered Lines: 5168
Relevant Lines: 8149
💛 - Coveralls

@mvomiero mvomiero marked this pull request as ready for review June 6, 2024 14:54
hohwille
hohwille approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@mvomiero Thank you for your PR. Great that you pro-actively found and fixed an important testing gap. Great work 👍
However, I am picky in PRs and if we do something I aim to do it perfect so please have a look at my review comments and try to address. Then we can merge. Thanks!

cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
cli/src/test/resources/ide-projects/mvn/project/settings/devon/conf/.m2/settings.xml Outdated Show resolved Hide resolved
cli/src/test/resources/ide-projects/mvn/project/settings/devon/conf/.m2/settings.xml Outdated Show resolved Hide resolved
cli/src/test/java/com/devonfw/tools/ide/tool/mvn/MvnTest.java Outdated Show resolved Hide resolved
@hohwille hohwille added this to the release:2024.06.002 milestone Jun 18, 2024
@mvomiero mvomiero closed this Jun 19, 2024
@mvomiero mvomiero deleted the enhancement/342-ImplementUnitTestsForMvn branch June 19, 2024 10:25
@mvomiero mvomiero restored the enhancement/342-ImplementUnitTestsForMvn branch June 19, 2024 10:26
@mvomiero mvomiero reopened this Jun 19, 2024
hohwille
hohwille approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@mvomiero thanks for addressing all review comments so perfectly 👍
Could you have a look at my last suggestion and resolve the merge conflict? Then we can finally merge. Great job!

cli/src/test/resources/ide-projects/mvn/project/workspaces/main/readme Outdated Show resolved Hide resolved
@mvomiero
Copy link
Contributor Author

mvomiero commented Jul 2, 2024

I added a review just to better explain the last changes 😃

hohwille
hohwille approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@mvomiero thanks for your rework and also for your fixes of Mvn commandlet that I was not yet aware of. Great work 👍
Now ready for merge.

@hohwille hohwille merged commit 2571a16 into devonfw:main Jul 4, 2024
3 checks passed
@hohwille hohwille added the reviewed Marks PRs that have been presented in the sprint-review meeting or that do not need to be presented. label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hohwille hohwille hohwille approved these changes

Assignees

@hohwille hohwille

@mvomiero mvomiero

Labels
mvn related to apache maven build tool reviewed Marks PRs that have been presented in the sprint-review meeting or that do not need to be presented.
Projects
IDEasy board
Status: ✅ Done
Milestone
release:2024.07.002
Development

Successfully merging this pull request may close these issues.

Implement unit tests for mvn ToolCommandlet
3 participants
@mvomiero @coveralls @hohwille