This repository was archived by the owner on Feb 2, 2021. It is now read-only.
Add getSpinner method to $progressIndicator #1040
Merged
Conversation
This file contains hidden or 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
| } else { | ||
| let msg = message; | ||
| return { | ||
| start: () => this.$logger.info(msg), |
There was a problem hiding this comment.
For cloud builds this will print for example Installing tns-android and we are going to loose the broken "progress indicator" which we currently have. Without it our users may wonder is the build slow or is there some other problem (network connectivity issue on their side, or some bug on our side).
Do you think it will be better to have some kind of progress indicator instead of just one message?
There was a problem hiding this comment.
I think we shouldn't rely on the cloud build logs in order to show progress indication on the client side. IMO this should be handled on the client side in any other meaningful way.
@tsvetie what do you think?
Add `getSpinner` method to $progressIndicator - it will return a new instance of clui.Spinner in case terminal is interactive. In case it is not - a mocked instance will be returned. This way in CI builds the spinner will not print tons of repeated messages.
TsvetanMilanov
approved these changes
Jan 16, 2018
Currently all http request can be piped in httpClient, which is uses when we want to download some file. Each of these pipes is passed through progressStream, so we can print some indication on the same line that something happens. We do not want this in CI environment. Also it is strange to print message like "Download completed" in the middle of any operation. So remove the progress indicator (in fact the percentage part of it has been broken for a while). Extract the method in helpers in case we need it in the future.
rosen-vladimirov
force-pushed
the
vladimirov/disable-spinners-in-ci
branch
from
4dc9e0c to
93db157
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Add
getSpinnermethod to$progressIndicator- it will return a new instance of clui.Spinner in case terminal is interactive. In case it is not - a mocked instance will be returned.This way in CI builds the spinner will not print tons of repeated messages.