-
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
#525 Added online state check before downloading #610
#525 Added online state check before downloading #610
Conversation
Pull Request Test Coverage Report for Build 11120017041Details
💛 - Coveralls |
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.
@WorkingAmeise thanks for your PR. You found the relevant spots in the code where to implement this feature. Also throwing CliOfflineException
is the perfect solution. 👍
You PR is functionally correct. I just left some comments for further thoughts and improvements. Please have a look.
cli/src/main/java/com/devonfw/tools/ide/repo/AbstractToolRepository.java
Outdated
Show resolved
Hide resolved
Can you please also check DoD: CHANGELOG entry needs to be added. |
…-disable-downloads-when-called-with-offline
…-disable-downloads-when-called-with-offline
…-disable-downloads-when-called-with-offline
…-disable-downloads-when-called-with-offline
…-disable-downloads-when-called-with-offline
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.
@WorkingAmeise thanks for your PR and your rework.
You addressed my review comment well and solved the centralization of messages in CliOfflineException
perfectly. 👍
Please remove CHANGELOG.adoc.orig
as commented and we can merge this PR.
closes #525
now Throws a CliOfflineException if the offline flag is set