pr create ignores base repository when selecting repository for pushing #800

Activity

1. fiam

   added

   bugSomething isn't workingSomething isn't working

   on Apr 17, 2020
2. billygriffin commented on Apr 17, 2020

   

   billygriffin

   on Apr 17, 2020

   Contributor

   More actions

   Hi @fiam, thanks for the feedback. This was an intentional change based on the feedback in #350. This is the first we've heard of the case of having write access to someone else's fork, but in that case, it does seem a bit odd. We're definitely not going to revert back to using the base repo if the user has write access for the reasons described in #350, but I'm wondering about the technical feasibility of this progression:

   1. Prefer your fork if you have one
   2. Upstream repo if you have write access and don't have a fork of your own

   Basically, this would ensure you're never pushing to someone else's fork unless you explicitly specify it.
3. fiam commented on Apr 18, 2020

   

   fiam

   on Apr 18, 2020

   Author

   More actions

   Hi @billygriffin. Looking at the code, the current behaviour seemed deliberate but I failed to find any relevant issues. Reading #350 and the linked issues (#172, #392, #464), it seems clear to me that no matter what default order you pick, at the end of the day there are going to be surprises and unexpected behaviors because there's no clear consensus on this matter.

   For example, the biggest project I'm contributing regularly at this time is INAV, a firmware for radio controlled vehicles. We do push our branches to the main repo because it makes collaborating between developers easier. Being a flight controller firmware, it's not uncommon for someone to develop a fix or a feature but lack some of the supported targets to test it (we support around 100 different boards at the moment, so no one has all of them) or don't have the time to do a long field test, so someone else might test it and add more commits on top. There are also users that help us field test PRs before it gets merged, and is easier for them to fetch the code if it's already on a branch in the main repo (I know and use the feature to fetch all open PRs, but that seems a bit too complicated some users). In fact, the reason someone else gave me write access to their fork was because they had submitted a PR that needed a few tweaks in some targets and I pushed my commits on top in their fork. I know and use other alternatives like git-am/git-apply, or pull from their repo, push to the main and ask them to pull from my branch on main - but in my experience not every developer knows about those tools/workflows and it's definitely more complicated than just pushing to their fork.

   This is also an interesting project for this example because for a long time I couldn't have a personal fork (I guess I could have worked around it by creating an org and forking there, but that seems a bit too complicated), because both INAV and Betaflight originated from Cleanflight and while I mainly contribute to INAV I also occasionally send patches to Betaflight (which I don't have write access to). Some time ago, the creator of the INAV project asked GH support to remove the "forked from Cleanflight" status, so we can now have personal forks for both. While this "multiple forks going on" is definitely an uncommon edge case, I just wanted to mention it because with the scale of GH you have so many projects and different use cases that even rare situations will present themselves and sometimes you have no choice but add explicit options so the user can tell you exactly what they want.

   Looking at popular projects with small-ish number of people with write access (like bootstrap, html5-boilerplate, ruby on rails, or font awesome), I can see most people with write access in those projects submit PRs from the main repository. On the other hand, you have projects that, for several reasons (hooks on branch creation, keeping only master and release branches on the main repo, etc...), have a policy to submit PRs from personal forks (note, however, that there's no GH setting to enforce this, or at least I can't find it).

   Going by my experience, I'd say pushing PRs to the main repo is more common but, as those aforementioned issues have shown, there's definitely a non negligible number of projects that want branches to be pushed to forks.

   Unfortunately, I don't think there's a clean way to reconcile both scenarios by guessing what the user intends. The solution you propose would definitely make things better than they are, but having to create or destroy a personal fork in order to make the gh pr create behave as you want seems like a huge hassle compared to a simple command line flag. I also think that once gh pr create has chosen a default remote for pushing, it shouldn't change unless the user explicitly ask for it. Silently changing the repo it pushes to because you created or deleted a personal fork would feel totally unexpected.

   The way I see, you have different options, sorted from nice to not-so-nice:

   1. Add a setting in the GH server side to disallow branch creation on push. Projects that don't want branches created on the main repo can enable this new setting and create the branches via web UI. gh can then read this setting from the API and act accordingly.

   2. When several available remotes with write access are found, ask the user manually to pick one, and remember it as the default. Add also a command line flag to select an specific remote, overwriting the default one. This also implies that if the user has write access to the main repo but no default has been chosen yet, you should ask them if they want to create a fork. This probably should be implemented even if you choose to go 1), because at the end of the day you could still find yourself in situations where you can create branches on multiple remotes but you do want an specific one (e.g. multiple separated forks - as in the classic meaning of "following a different line of development and managed by other people" - from the same ancestor project, and a user contributing to both of them).

   3. Remove the ability for pushing from gh pr create and ask users to explicitly push before creating the PR, it's just a git command which can also be configured/aliased to do exactly what you want with minimal typing.

   4. Leave repo selection as it is, but print the repo the branch will be pushed to, so users have a chance to abort and push manually to override the default.
4. mislav commented on Apr 20, 2020

   

   mislav

   on Apr 20, 2020

   Last edited by billygriffin

   Contributor

   More actions

   @fiam Thank you for detailing your use case and proposing potential fixes!

   We intentionally allowed auto-pushing to someone else's fork where you have write access mainly to support organization forks. Example: I work for GitHub where we forked someone/example-lib as github/example-lib and where my coworkers and I make our patches and occasionally submit them upstream to someone. Because there is already a fork where I have push access, CLI assumes that there is no need to create my own personal fork.

   As with all assumptions and as your use-case demonstrates, this assumption is sometimes wrong. We will re-evaluate our current approach, but in the meantime, you can always choose a push target by first pushing with git:

   git push -u <remote> HEAD
   gh pr create  #=> should never auto-fork nor auto-push

   If a manual push to a specific remote ever isn't respected as PR head, please open a separate issue. 🙇
5. prestonvanloon commented on Jul 8, 2020

   

   prestonvanloon

   on Jul 8, 2020

   More actions

   I also have this use case. I would like to set the default to create PRs in the origin, rather than in a fork.

   Using the workaround in #800 (comment) works, but I'd really like to set this behavior as the default.

   👍React with 👍1Reacted by David Salgado
6. mislav

   closed this as completedin #1706on Sep 16, 2020