Skip to content

Conversation

@probablycorey
Copy link
Contributor

@probablycorey probablycorey commented May 1, 2020

Relies on #854

Here is what it looks like

CleanShot 2020-05-01 at 14 14 21@2x

This command isn't as straight forward as that though because the PR could be merged. If it is we show the message below.

CleanShot 2020-05-01 at 14 09 20@2x

Closes #413


if pr.State == "MERGED" {
fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d can't be reopened because it was already merged.\n", utils.Red("!"), pr.Number)
return nil
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if this should return an error or not. What do you think @vilmibm @mislav?

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I think we should fail with a non-zero status every time we are unable to perform an operation, or (more precisely) if we are unable to reach the end-state that the user requested.

@probablycorey probablycorey self-assigned this May 1, 2020
@probablycorey probablycorey marked this pull request as ready for review May 1, 2020 21:18
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.

🚀🚀💫


if pr.State == "MERGED" {
fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d can't be reopened because it was already merged.\n", utils.Red("!"), pr.Number)
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I think we should fail with a non-zero status every time we are unable to perform an operation, or (more precisely) if we are unable to reach the end-state that the user requested.

return err
}

func PullRequestReopen(client *Client, repo ghrepo.Interface, pr PullRequest) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since other arguments to this function are pointers, perhaps we could accept a pointer to *PullRequest as well? With that we could avoid de-referencing *pr at call point

@probablycorey probablycorey merged commit 4020084 into close-a-pull-request May 4, 2020
@probablycorey probablycorey deleted the reopen-pr branch May 4, 2020 17:42
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.

3 participants