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
feat: Add support for negative default in confirm prompt, fixes #6103 #6118
feat: Add support for negative default in confirm prompt, fixes #6103 #6118
Conversation
- Confirm is updated to a wrapper of ConfirmTo to preserve current behavior - Updates comments accordingly
Thanks! |
Download the artifacts for this pull request:
See Testing a PR. |
Since Confirm is just a wrapper, its possible values have already been accounted for in TestConfirmTo.
I've added the test for the new function and this should be ready for review. I wasn't too happy with the name of the function. My initial thought was to simply make the default value optional in the same Confirm function but that's not a thing in go so I had to go this route. Happy to get some thoughts or feedback! |
@cballenar, glad you're working on it, I've run tests and it needs formatting. Please install
|
Thanks @stasadev ! Linting issues have been resolved now. |
Will eventually get caught up to review, thanks for your patience! |
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.
Thank you @cballenar, especially for the elegant refactoring for the tests!
I checked ddev push
manually, the default answer is no.
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.
This works great, thanks so much, and thanks for your patience!
The Issue
Sensitive operations such as
ddev provider push
recommend being careful but default to a 'yes' value if left blank.How This PR Solves The Issue
We add a utility,
ConfirmTo(prompt, default)
which takes the logic ofConfirm(prompt)
and allows passing a default value (of true or false).We then use
ConfirmTo
in the push command and default tofalse
.Lastly,
Confirm
is updated to be a wrapper ofConfirmTo
, thus maintaining current behavior without repeating code.Manual Testing Instructions
(y/N)
and if blank is submitted it will cancel the operationAutomated Testing Overview
Replaces
TestConfirm
withTestConfirmTo
. SinceConfirm
is just a wrapper, its possible values have already been accounted for inTestConfirmTo
.Related Issue Link(s)
#6103
Release/Deployment Notes