`pr create` - implement --push-to-base-repo flag by arturhoo · Pull Request #8146 · cli/cli

arturhoo · 2023-10-05T13:05:01Z

My workflow with private repositories usually consists of:

Create a new branch locally
Modify code and create commits
Create a PR in the private repository using gh repo create --fill

The vast majority of times, the PR will be created in the base repository as defined by

Line 60 in 3443a75

    
           func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, error) {

In other words, I'm pushing my local branch to the origin remote (the central, private repository) as part of the PR creation flow.

This is currently done through an interactive prompt, where the base repository is presented as the first option - I've created the muscle memory of just pressing <enter> right after running gh pr create every time.

This PR introduces a new flag --push-to-base-repo that skips this interactive prompt, when the user has write access to that repository.

Fixes #8152

--

P.S.: this seems to have been discussed in #1718 - I think this approach might tackle at least the common workflow os folks who have clones a repository which they have write access to (and don't submit PRs through forks).

cliAutomation · 2023-10-05T13:14:40Z

Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message.

andyfeller

@arturhoo : Thank you for opening up this pull request, working to improve the GitHub CLI!

I can see how this is tangential to #1718, however I think it would best to create an issue especially for this work that the current core contributors can discuss as that issue is over 3 years old.

andyfeller · 2023-10-09T13:28:26Z

pkg/cmd/pr/create/create.go

+	if headRepo == nil && isPushEnabled && opts.PushToBaseRepo {
+		pushableRepos, err := repoContext.HeadRepos()
+		if err != nil {
+			return nil, err
+		}
+
+		for _, r := range pushableRepos {
+			if ghrepo.IsSame(baseRepo, r) {
+				headRepo = r
+			}
+		}
+
+		if headRepo == nil {
+			return nil, fmt.Errorf("the user might lack write access to the base repository")
+		}
+	}
+


Should this be refactored into the following section to avoid duplicating logic but leveraging (r *ResolvedRemotes) RemoteForRepo, which is doing similar logic already?

cli/context/context.go

Lines 161 to 169 in 3443a75

// RemoteForRepo finds the git remote that points to a repository

func (r *ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) {

for _, remote := range r.remotes {

if ghrepo.IsSame(remote, repo) {

return remote, nil

}

}

return nil, errors.New("not found")

}

I'm probably missing something, but I don't see how we could incorporate the logic into the (r *ResolvedRemotes) RemoteForRepo method.

The proposed logic iterates over all pushable repos and determines if baseRepo can be pushed, just to fail early. Remotes, which RemoteForRepo handles, will only be used by the time we actually push on

cli/pkg/cmd/pr/create/create.go

Lines 741 to 743 in 363dacb

if headRemote == nil && headRepo != nil {

headRemote, _ = ctx.RepoContext.RemoteForRepo(headRepo)

}

andyfeller · 2023-10-09T13:29:57Z

pkg/cmd/pr/create/create.go

 	fl.Bool("no-maintainer-edit", false, "Disable maintainer's ability to modify pull request")
 	fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create")
 	fl.StringVarP(&opts.Template, "template", "T", "", "Template `file` to use as starting body text")
+	fl.BoolVar(&opts.PushToBaseRepo, "push-to-base-repo", false, "Push the head branch to the base repository")


If we're going to expand flags to allow people to determine this from the command line, knowing there are more options that people want, then I imagine this might be more generic flag with different values for prompt, base, ...

Interesting idea! I took a look at the code, but I couldn't find any other 'canonical' repository names except for base (which already has an implementation for that we're using).

For prompt, that's already the default behaviour if this flag isn't passed

cli/pkg/cmd/pr/create/create.go

Line 577 in 363dacb

// otherwise, ask the user for the head repository using info obtained from the API

Are you suggesting accepting any value here and validating it against the pushableRepos as seen on https://github.com/arturhoo/cli/compare/push-to-base-repo...arturhoo:cli:push-to-repo?expand=1 ?

arturhoo · 2023-10-11T16:11:38Z

Hey @andyfeller - thank you for taking the time to review my suggestion. I've replied to each suggestion individually and created a new issue #8152 - I would appreciate if you could take another look before I make further changes. Thanks!

samcoe · 2023-10-17T09:17:24Z

Closing, see #8152 (comment) for context.

arturhoo force-pushed the push-to-base-repo branch from 6715a1a to 5d63839 Compare October 5, 2023 13:13

arturhoo marked this pull request as ready for review October 5, 2023 13:14

arturhoo requested a review from a team as a code owner October 5, 2023 13:14

arturhoo requested review from andyfeller and removed request for a team October 5, 2023 13:14

cliAutomation added the external pull request originating outside of the CLI core team label Oct 5, 2023

arturhoo changed the title ~~cmd/pr/create - implement --push-to-base-repo flag~~ pr create - implement --push-to-base-repo flag Oct 5, 2023

andyfeller suggested changes Oct 9, 2023

View reviewed changes

andyfeller added enhancement a request to improve CLI core This issue is not accepting PRs from outside contributors gh-pr relating to the gh pr command labels Oct 9, 2023

arturhoo mentioned this pull request Oct 9, 2023

pr create: flag to push local branch to remote without prompting #8152

Open

cmd/pr/create - implement --push-to-base-repo flag

db491aa

arturhoo force-pushed the push-to-base-repo branch from 5d63839 to db491aa Compare October 11, 2023 16:20

arturhoo requested a review from andyfeller October 17, 2023 07:14

samcoe closed this Oct 17, 2023

danp mentioned this pull request Mar 20, 2024

Allow setting a default push target for pr create #1718

Open

lzm0 mentioned this pull request May 15, 2025

Support auto-selecting push target for gh pr create #10988

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`pr create` - implement --push-to-base-repo flag#8146

`pr create` - implement --push-to-base-repo flag#8146
arturhoo wants to merge 1 commit intocli:trunkfrom
arturhoo:push-to-base-repo

arturhoo commented Oct 5, 2023 •

edited

Loading

Uh oh!

cliAutomation commented Oct 5, 2023

Uh oh!

andyfeller left a comment

Uh oh!

andyfeller Oct 9, 2023

Uh oh!

arturhoo Oct 11, 2023

Uh oh!

andyfeller Oct 9, 2023

Uh oh!

arturhoo Oct 11, 2023

Uh oh!

arturhoo commented Oct 11, 2023

Uh oh!

samcoe commented Oct 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

	// RemoteForRepo finds the git remote that points to a repository
	func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (Remote, error) {
	for _, remote := range r.remotes {
	if ghrepo.IsSame(remote, repo) {
	return remote, nil
	}
	}
	return nil, errors.New("not found")
	}

	if headRemote == nil && headRepo != nil {
	headRemote, _ = ctx.RepoContext.RemoteForRepo(headRepo)
	}

Conversation

arturhoo commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cliAutomation commented Oct 5, 2023

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

andyfeller Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

arturhoo Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

andyfeller Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

arturhoo Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

arturhoo commented Oct 11, 2023

Uh oh!

samcoe commented Oct 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

arturhoo commented Oct 5, 2023 •

edited

Loading