Add prompt to delete local branch when attempting to merge a PR that … #2789
Conversation
…is already merged
pkg/cmd/pr/merge/merge.go
Outdated
| } | ||
| } else { | ||
| err := prompt.SurveyAskOne(&survey.Confirm{ | ||
| Message: fmt.Sprintf("Pull request #%d (%s) was already merged. Would you like to delete this local branch and switch to the default branch?", pr.Number, pr.Title), |
There was a problem hiding this comment.
If we are not in interactive mode, this prompt should never happen. Additionally, if the user supplied --delete-branch, this prompt should never happen either — we can just continue in that case.
If you'd like, you could try additionally addressing #2715 in this PR! It will touch on the same logic.
There was a problem hiding this comment.
Thanks for the feedback! I'll make those adjustments and look at that other issue. I might not get to it until Monday.
One other question I had for you is do you this the prompt message is too wordy/confusing? After I opened this PR I thought about it more and maybe something like ... Delete the branch locally and switch to default branch? would be more appropriate. I find my use of the word 'this' confusing when you consider merging a PR by number from another branch because in that case you aren't deleting 'this' branch.
There was a problem hiding this comment.
@dpromanko I like your proposed change with "...delete the branch locally and..." instead of the current verbiage.
dpromanko
force-pushed
the
dpromanko/2625
branch
from
8289a55
to
40e9b57
Compare
…hen -d flag is passed
dpromanko
force-pushed
the
dpromanko/2625
branch
from
40e9b57
to
df31fae
Compare
…is already merged
resolves #2625, resolves #2715