diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 76f4031d75d..4ff7fbca27a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -2,19 +2,19 @@ Hi! Thanks for your interest in contributing to the GitHub CLI! -We accept pull requests for bug fixes and features where we've discussed the approach in an issue and given the go-ahead for a community member to work on it. We'd also love to hear about ideas for new features as issues. +We accept pull requests for issues labelled `help wanted`. We encourage issues and discussion posts for all other contributions. ### Please do: * Check issues to verify that a [bug][bug issues] or [feature request][feature request issues] issue does not already exist for the same problem or feature * Open an issue if things aren't working as expected -* Open an issue to propose a significant change +* Open an issue to propose a change * Open an issue to propose a design for an issue labelled [`needs-design` and `help wanted`][needs design and help wanted], following the [proposing a design guidelines](#proposing-a-design) instructions below * Open an issue to propose a new community supported `gh` package with details about support and redistribution * Mention `@cli/code-reviewers` when an issue you want to work on does not have clear Acceptance Criteria * Open a pull request for any issue labelled [`help wanted`][hw] and [`good first issue`][gfi] -### Please _do not_: +### Please _do NOT_: * Open a pull request for issues without the `help wanted` label or explicit Acceptance Criteria * Expand pull request scope to include changes that are not described in the issue's Acceptance Criteria diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 705f460232b..5b5d45f9c0a 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -166,6 +166,16 @@ func (r forkableRefs) UnqualifiedHeadRef() string { return r.qualifiedHeadRef.BranchName() } +// isSameRef checks if the head and base refs point to the same ref in the same repository. +// For cross-repository PRs (e.g., from a fork), the qualified head ref will contain +// an owner prefix (owner:branch), so even if branch names match, they refer to different repos. +func isSameRef(refs creationRefs) bool { + if strings.Contains(refs.QualifiedHeadRef(), ":") { + return false + } + return refs.UnqualifiedHeadRef() == refs.BaseRef() +} + // CreateContext stores contextual data about the creation process and is for building up enough // data to create a pull request. type CreateContext struct { @@ -367,6 +377,10 @@ func createRun(opts *CreateOptions) error { return err } + if isSameRef(ctx.PRRefs) { + return fmt.Errorf("head branch %q is the same as base branch %q, cannot create a pull request", ctx.PRRefs.UnqualifiedHeadRef(), ctx.PRRefs.BaseRef()) + } + httpClient, err := opts.HttpClient() if err != nil { return err diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 9af35183015..5ccffd7a8a8 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -384,6 +384,31 @@ func Test_createRun(t *testing.T) { }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", }, + { + name: "same head and base branch should error", + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.TitleProvided = true + opts.BodyProvided = true + opts.Title = "my title" + opts.Body = "my body" + opts.HeadBranch = "master" + return func() {} + }, + wantErr: `head branch "master" is the same as base branch "master", cannot create a pull request`, + }, + { + name: "same head and base branch with explicit base should error", + setup: func(opts *CreateOptions, t *testing.T) func() { + opts.TitleProvided = true + opts.BodyProvided = true + opts.Title = "my title" + opts.Body = "my body" + opts.HeadBranch = "feature" + opts.BaseBranch = "feature" + return func() {} + }, + wantErr: `head branch "feature" is the same as base branch "feature", cannot create a pull request`, + }, { name: "dry-run-nontty-with-default-base", tty: false, @@ -2879,3 +2904,73 @@ func TestProjectsV1Deprecation(t *testing.T) { }) }) } + +func Test_isSameRef(t *testing.T) { + tests := []struct { + name string + refs creationRefs + expected bool + }{ + { + name: "same branch in same repo", + refs: skipPushRefs{ + qualifiedHeadRef: shared.NewQualifiedHeadRefWithoutOwner("main"), + baseRefs: baseRefs{ + baseBranchName: "main", + }, + }, + expected: true, + }, + { + name: "different branches in same repo", + refs: skipPushRefs{ + qualifiedHeadRef: shared.NewQualifiedHeadRefWithoutOwner("feature"), + baseRefs: baseRefs{ + baseBranchName: "main", + }, + }, + expected: false, + }, + { + name: "same branch name in different repos (cross-repo PR)", + refs: skipPushRefs{ + qualifiedHeadRef: shared.NewQualifiedHeadRef("other-owner", "main"), + baseRefs: baseRefs{ + baseBranchName: "main", + }, + }, + expected: false, + }, + { + name: "pushableRefs same branch same repo", + refs: pushableRefs{ + headRepo: ghrepo.New("OWNER", "REPO"), + headBranchName: "main", + baseRefs: baseRefs{ + baseRepo: api.InitRepoHostname(&api.Repository{Name: "REPO", Owner: api.RepositoryOwner{Login: "OWNER"}}, "github.com"), + baseBranchName: "main", + }, + }, + expected: true, + }, + { + name: "pushableRefs same branch different repos (fork)", + refs: pushableRefs{ + headRepo: ghrepo.New("FORK-OWNER", "REPO"), + headBranchName: "main", + baseRefs: baseRefs{ + baseRepo: api.InitRepoHostname(&api.Repository{Name: "REPO", Owner: api.RepositoryOwner{Login: "OWNER"}}, "github.com"), + baseBranchName: "main", + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isSameRef(tt.refs) + assert.Equal(t, tt.expected, result) + }) + } +}