1

So I recently read this blog post on stacked diffs for Github PR's and have been trying to follow it. I've run into a snag though for a repo I'd like to make a few stacked PR's against for which I don't have access to the upstream repo so I can't push any branches to it. I created an initial branch in my forked repo and opened up a PR against master of the upstream repo. I then created another branch with that previous branch as my base. I made some code changes and I wanted to open a second PR with my first branch as base but I can only do so against my own forked repo and not the upstream repo. Has anyone ever run into a similar problem? In the past I've had access to the upstream repo so this hasn't been a problem because I could push my branches to upstream. I'm just not sure of a possible workaround for the case where you don't have access to upstream.

jeromefroe
  • 1,345
  • 12
  • 19
  • If I understand it correctly you created a branch of your fork and then created a PR from that to master. If you do this there is no 'new' branch on upstream but only on your fork. I didn't quite get the second part of the question, as in why do you want to merge new changes to new branch? If you made more changes which are for that branch then commit on the same branch on your fork and ur PR will be updated or create a new PR (if old PR is closed) or better make a new branch (based out of old PR / master if prev. PR was merged) on your fork and create a new PR – Bhupendra Feb 15 '17 at 04:52
  • So I want to merge the new changes into upsteam master, but to help make the PR's more readable I'd like to have multiple PR's, each of which builds on top of the last. So, for example, the first PR might introduce the interfaces I intend to use, the second PR might introduce concrete implementations of those interfaces, and the third PR might use those implementations somewhere else in the repo. Therefore, I don't want to add my changes to my first PR because that will make one big PR. On the other hand, if I create a new PR from my new branch it will include the changes in my first PR. – jeromefroe Feb 15 '17 at 12:30

1 Answers1

2

I then created another branch with that previous branch as my base. I made some code changes and I wanted to open a second PR with my first branch as base but I can only do so against my own forked repo and not the upstream repo

Yes, that is expected: you compare your work against an upstream branch, and yours has not yet been merged in the original repo: it exists only in your fork.

A workaround is to:

  • create your second branch from upstream/master (meaning from a branch of the original repo)
  • git cherry-pick (which is to say duplicate) the commits from your first branch,
  • add your work for your second branch and push that to your fork
  • make your PR from your fork

As the OP concludes:

it didn't quite accomplish what I had in mind as the PR included the changes from the first branch.

I believe the problem is ultimately that Github doesn't let me make the base of a PR against upstream a branch in a forked repo for which a PR is already open.

This of course makes sense as that branch hasn't been merged into upstream yet and so doesn't exist in the upstream repo, but it does seem to proscribe the possibility of stacked pull requests for an upstream repo which one cannot push to.

I agree: a PR, ideally, is meant to be applied independently of other PRs: the maintainer pick and chose which ones to accepts, which ones to rejects.

What I described above is a workaround, but ultimately is not a best practice.

The OP adds:

What I decided to do was open my first PR against upstream master and my subsequent PR's against the previous branch in my forked repo.
Then, as one PR lands, I can easily change the base branch for the next PR to be upstream master which now includes the changes from my previous PR.

That is indeed the correct way to update an existing PR against an updated origin/master (which now includes another merged PR)

Community
  • 1
  • 1
VonC
  • 1,262,500
  • 529
  • 4,410
  • 5,250
  • Thanks for the suggestion! I'll give it a try. It seems like this might run into a problem though because the second branch I create from upstream/master in the first step will only exist locally. Therefore, I won't be able to use it as the base for my PR. Haven't tried it yet though so I might be missing something. – jeromefroe Feb 15 '17 at 12:36
  • @jeromefroe yes, to make a PR, you will need to push your second branch first from your local repo to your fork. That is what I meant by the "push that to your fork" in my answer. – VonC Feb 15 '17 at 12:39
  • I tried your workaround and it didn't quite accomplish what I had in mind as the PR included the changes from the first branch. I believe the problem is ultimately that Github doesn't let me make the base of a PR against upstream a branch in a forked repo for which a PR is already open. This of course makes sense as that branch hasn't been merged into upstream yet and so doesn't exist in the upstream repo, but it does seem to proscribe the possibility of stacked pull requests for an upstream repo which one cannot push to. – jeromefroe Feb 15 '17 at 14:31
  • @jeromefroe I agree: a PR, ideally, does not depend on another non-merged PR. – VonC Feb 15 '17 at 15:07
  • @jeromefroe i have included your comment in the answer, to emphasize what is the best practice and why. – VonC Feb 15 '17 at 15:09
  • I'm not sure I agree generally with the statement that "a PR, ideally, does not depend on another non-merged PR" as I think the stacked diff approach to pull requests is a productive one. Also, this approach works quite well for repos for which I can push branches to the upstream repo. I agree that it does falls apart for PR's from forked repos, however. – jeromefroe Feb 15 '17 at 15:28
  • For those who are interested, what I decided to do was open my first PR against upstream master and my subsequent PR's against the previous branch in my forked repo. Then, as one PR lands, I can easily change the base branch for the next PR to be upstream master which now includes the changes from my previous PR. – jeromefroe Feb 15 '17 at 15:29
  • @jeromefroe Unfortunately, the "stacked diff approach to pull requests" is *not* a productive one for the maintainer (the one in charge of merging your PR), as it would force him/her to understand which PR depends on which: his/her job is right now much simpler: he/she picks whatever PR he/she wants and tries it out. – VonC Feb 15 '17 at 15:29
  • @jeromefroe I fully agree with your second comment and have included it in the answer: rebasing a PR against an updated origin/master is the right solution. You can the push --force your second PR: it will be updated. – VonC Feb 15 '17 at 15:32
  • I think that with appropriate labeling and metadata it can be productive for the maintainer as well as it makes each code review smaller and more focused. Good titles for the PR go a long way. For example "[Add feature X - PART 1] Add initial interfaces" is a lot better than "Add interfaces for feature X". This way it is clear what the main goal of the PR's is, what each individual PR is trying to accomplish, and where each PR falls in the sequence of PR's. – jeromefroe Feb 15 '17 at 15:44
  • @jeromefroe I agree, but this is clearly *not* what it proposed/managed today by the GitHub GUI interface. Hence my answer above. – VonC Feb 15 '17 at 15:54
  • very true. Anyway, thanks for your feedback, it's been extremely helpful! – jeromefroe Feb 15 '17 at 16:25