Skip to content

Normalize pr command arguments#3024

Merged
samcoe merged 2 commits intotrunkfrom
normalize-pr-commands
Feb 26, 2021
Merged

Normalize pr command arguments#3024
samcoe merged 2 commits intotrunkfrom
normalize-pr-commands

Conversation

@samcoe
Copy link
Contributor

@samcoe samcoe commented Feb 23, 2021

For this PR I went through and normalized all relevant PR command arguments. They should now be consistent across the whole set. They all will take 0 or 1 arguments, where 0 means we will determine the PR from the currently checked out git branch.

closes #3003

@samcoe samcoe self-assigned this Feb 23, 2021
@samcoe samcoe requested a review from a team February 23, 2021 17:22
Use: "close {<number> | <url> | <branch>}",
Use: "close [<number> | <url> | <branch>]",
Short: "Close a pull request",
Args: cobra.ExactArgs(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

This might have not been apparent, but we wanted to force the user to explicitly supply the argument to close instead of taking the current PR implicitly. This made it harder to close the current PR by accident, even though it was inconsistent with other pr commands where the argument was optional.

Use: "reopen {<number> | <url> | <branch>}",
Use: "reopen [<number> | <url> | <branch>]",
Short: "Reopen a pull request",
Args: cobra.ExactArgs(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as close: we wanted to force the explicit argument because of the destructiveness of close/reopen.

@samcoe
Copy link
Contributor Author

samcoe commented Feb 23, 2021

@mislav Thanks for the context around close and reopen. I removed those changes so now only edit and comment have been normalized.

@samcoe samcoe requested a review from mislav February 23, 2021 21:26
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

This looks good!

BTW see the new helper that just landed in trunk #3021

@samcoe samcoe merged commit a496549 into trunk Feb 26, 2021
@samcoe samcoe deleted the normalize-pr-commands branch February 26, 2021 18:31
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.

Make pr edit dynamically look up PR based on current branch

2 participants