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

Actually default-yes for installing dashboard #2849

Merged
merged 1 commit into from
Oct 12, 2022
Merged

Conversation

ozamosi
Copy link
Contributor

@ozamosi ozamosi commented Oct 11, 2022

The instructions and the implementation didn't line up - we claimed yes was default, but just hitting enter caused the value to be "" which meant we didn't actually install it.

This is caused by using IsConfirm, where it doesn't set the default as the default value, but rather uses the error value as nil if the user agreed, and promptui.ErrAbort (which is errors.New("")) if the user didn't.

@ozamosi ozamosi requested a review from a team October 11, 2022 13:42
@ozamosi ozamosi force-pushed the fix-dashboard-default branch 2 times, most recently from 20c2f1d to b23a3bd Compare October 11, 2022 13:59
@opudrovs
Copy link
Contributor

In the description of create dashboard for gitops run issue the description was "If the user answers yes, let them enter a password and use that for the installation". So, I supposed yes was not a default value and the user had to agree to installing the dashboard (to "answer yes") explicitly? Or where did we list yes as the default value?

@opudrovs
Copy link
Contributor

Or was it somewhere in the prompts?

@ozamosi
Copy link
Contributor Author

ozamosi commented Oct 11, 2022

The code says Default: "Y", so it prints [Y/n] - I don't particularly care whether yes or no is the default, but the code misunderstands the return value.

@opudrovs
Copy link
Contributor

Ah, I see. Good catch will test it later.

@opudrovs
Copy link
Contributor

Tested and confirmed the issue. isConfirm sets input to "" in the promptui code:

var inputErr error
input := p.Default
if p.IsConfirm {
    input = ""
}

@ozamosi ozamosi force-pushed the fix-dashboard-default branch from b23a3bd to 81612c6 Compare October 12, 2022 09:13
The instructions and the implementation didn't line up - we claimed
yes was default, but just hitting enter caused the value to be ""
which meant we didn't actually install it.

This is caused by using `IsConfirm`, where it doesn't set the default
as the default value, but rather uses the error value as `nil` if the
user agreed, and promptui.ErrAbort (which is `errors.New("")`) if the
user didn't.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants