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.

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

fix(network): spinner issue for validator and coordinator commands #2939

Merged
merged 10 commits into from
Oct 18, 2022

Conversation

lumtis
Copy link
Contributor

@lumtis lumtis commented Oct 17, 2022

Add cliui.StartSpinner() to cliui.New

session := cliui.New(cliui.StartSpinner())

This fix the issue highlighted here: #2920 (comment)

If the option is not provided, spinner in the session object of the high level method will be nil and therefore will not stop the spinner used in the inside methods.
I'm wondering if this is not a misconception of cliui, should I open an issue for this?

@lumtis lumtis added the skip-changelog Don't check changelog for new entries label Oct 17, 2022
@tbruyelle
Copy link
Contributor

I think the fix should go to cliui indeed, by using session pointers everywhere needed.

@lumtis
Copy link
Contributor Author

lumtis commented Oct 18, 2022

I think the fix should go to cliui indeed, by using session pointers everywhere needed.

#2941

We can still merge this one as a quick fix

@lumtis lumtis requested a review from tbruyelle October 18, 2022 14:30
jeronimoalbi
jeronimoalbi previously approved these changes Oct 18, 2022
ignite/cmd/network_coordinator_set.go Outdated Show resolved Hide resolved
ignite/cmd/network_validator_set.go Outdated Show resolved Hide resolved
tbruyelle
tbruyelle previously approved these changes Oct 18, 2022
@lumtis lumtis dismissed stale reviews from tbruyelle and jeronimoalbi via 9aa949b October 18, 2022 20:48
@lumtis lumtis merged commit 7996b0a into develop Oct 18, 2022
@lumtis lumtis deleted the fix/network-spinner branch October 18, 2022 21:23
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
…gnite#2939)

* fix 1

* fix 2

* format

* remove unnecessary

* imports

Co-authored-by: Alex Johnson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Don't check changelog for new entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants