-
Notifications
You must be signed in to change notification settings - Fork 23
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.
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
#118: smart git pulls #163
Conversation
Pull Request Test Coverage Report for Build 7709855730
💛 - Coveralls |
Please note that this PR will conflict with PR #155 whatever may be merged first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
I just added some small suggestions that could be worked in.
cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salimbouch thanks for your PR and the nice working solution. 👍
Sorry for the long review delay.
It is great that you added a new gitPullOrCloneIfNeeded
method to add the new behavior on top. Your implementation is functionally perfectly correct.
I added some comment for improvement. Can you resolve merge conflict and address the review comments? Then we are ready to merge this nice change.
closes #118