Conversation
There was a problem hiding this comment.
Thanks! This is a welcome addition, but it could use some tests please.
Also, when opening a PR for a bug that hasn't yet been filed as an issue, please open an issue first and then reference that issue number in your pull request body with a message such as Fixes #123. This helps us track bugs.
In this case, I will open an issue for this based on what your PR fixes.
command/pr_create.go
Outdated
| return fmt.Errorf("must be on a branch named differently than %q", baseBranch) | ||
| } | ||
|
|
||
| _, err = api.PullRequestForBranch(client, headRepo, headBranch) |
There was a problem hiding this comment.
Shouldn't we query baseRepo here, since the "base" repository is the one where all the PRs are listed in?
| _, err = api.PullRequestForBranch(client, headRepo, headBranch) | |
| _, err = api.PullRequestForBranch(client, baseRepo, headBranch) |
command/pr_create.go
Outdated
| } | ||
|
|
||
| _, err = api.PullRequestForBranch(client, headRepo, headBranch) | ||
| if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { |
There was a problem hiding this comment.
I'm worried that this check will fail if we ever tweak the text of the original error message.
To reliably detect the type of an error, I think that PullRequestForBranch() should return a special type for the "not found" case, perhaps one named ErrNotFound. Then we could do this:
| if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { | |
| if !errors.Is(err, api.ErrNotFound) { |
How does this sound?
There was a problem hiding this comment.
I think, we can reuse api.NotFoundError here
command/pr_create.go
Outdated
|
|
||
| _, err = api.PullRequestForBranch(client, headRepo, headBranch) | ||
| if err != fmt.Errorf("no open pull requests found for branch %q", headBranch) { | ||
| return fmt.Errorf("pull request for branch %q already exists", headBranch) |
There was a problem hiding this comment.
Should we perhaps print the URL to the existing pull request in this error message or at least instruct the viewer that they can open the existing PR with gh pr view?
3 participants
Fixes #649