Skip to content

use Prompter in pr package - #6451

Merged
vilmibm merged 5 commits into
trunkfrom
pr-prompter
Dec 5, 2022
Merged

vilmibm merged 5 commits into
trunkfrom
pr-prompter

Conversation

@vilmibm

@vilmibm vilmibm commented Oct 18, 2022 •

Copy link
Copy Markdown
Contributor

This PR converts the code in pkg/cmd/pr to use the new Prompter. I did not port the metadata UI, however, as I want to wait until I've written better testing utils for Prompter.

vilmibm added 4 commits October 19, 2022 13:05
Note that this isn't done; it's leaving the metadata piece alone until
better testing utils are in place
@vilmibm vilmibm changed the title [wip] use Prompter in pr commands use Prompter in pr package Oct 19, 2022
@vilmibm
vilmibm marked this pull request as ready for review October 19, 2022 20:06
@vilmibm
vilmibm requested a review from a team as a code owner October 19, 2022 20:06
@vilmibm
vilmibm requested review from samcoe and removed request for a team October 19, 2022 20:06

@mislav mislav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

},
askStubs: func(as *prompt.AskStubber) {
as.StubPrompt("Where should we push the 'feature' branch?").
AssertOptions([]string{"OWNER/REPO", "Create a fork of OWNER/REPO", "Skip pushing the branch", "Cancel"}).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to verify all possible options shown to the user, and the new assertion doesn't do that anymore.

I do not think that every test should assert all available options, but it would be nice if at least some tests did.

@vilmibm
vilmibm enabled auto-merge December 5, 2022 19:34
@vilmibm
vilmibm merged commit a22b7ca into trunk Dec 5, 2022
@vilmibm
vilmibm deleted the pr-prompter branch December 5, 2022 19:43
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.

4 participants