7

Our team uses Github Pull Requests to manage our workflow, much like what is described here. Upon manually reviewing the accepted Pull Request, we occasionally need to revert that merge because it isn't ready for deployment to our production servers.

However, if a developer attempts to issue a Pull Request again, it does not recognize these changes were reverted and sees that the commits are already in the master branch. It only will include their recent commits since the revert, but what we really want is to reintroduce ALL of the commits there were reverted, plus their new work. In other words, we like a way to reissue the original Pull Request.

Since Github doesn't support this feature (i.e., neither reverting a merge, nor undoing/reissuing an original pull request), I am currently reverting the reverted merge. This feels wrong.

What other ways could I use to achieve the same goal in git? (or Github if it's possible)

svick
  • 236,525
  • 50
  • 385
  • 514
Chip Castle
  • 2,132
  • 3
  • 20
  • 34
  • 1
    If you've locally tried merged the commits from the pull request, and decided after testing that you don't want to do that merge yet, why do you revert the merge, rather than just resetting master back to before the merge? (I assume you don't publish your master branch after merging a pull request but before deciding whether to keep it or not.) – Mark Longair Nov 01 '11 at 16:41
  • Once the Pull Request is accepted, it's automatically merged into master, so anyone on our team can pull from there anytime. By reverting, I was following the advice of the blog post I referenced in my question, because it allowed us to simply move on to other Pull Requests and minimize bottlenecks in our workflow. I'm concerned that resetting would make matters worse due to the fact that master is always available to our repo collaborators. – Chip Castle Nov 01 '11 at 18:15
  • Ah, so you're accepting the pull request actually on GitHub. (The feature to ask GitHub to actually do the merge was added quite recently.) Instead, I would fetch the suggested commits into your local repository, merge them and test there. If you're happy with that, then you can mark the pull request as accepted on GitHub. – Mark Longair Nov 01 '11 at 21:32
  • Mark - you have a good point. I'm a little hesitant to add layers to our process, as it used to be very heavy and as a result, development grinded to a halt. However, you gave me an idea: I could setup Jenkins to run CI specs if anyone pushes a feature branch to one of the development servers we rarely use. Then I could prevent some issues from ever creeping into master. However, there will STILL be times when I need to revert or undo an operation, so I'm still digging for an answer. Thanks for your help. – Chip Castle Nov 02 '11 at 00:50
  • Now I understand this a better I've expanded my answer and undeleted it. I'd agree that, if you can, using Jenkins to test feature branches is a good idea. – Mark Longair Nov 02 '11 at 13:55

3 Answers3

1

I think that your problem here arises because when you are dealing with the pull requests, you're choosing to automatically merge them on GitHub. Out of the three suggested ways of dealing with pull requests described in the documentation you're using the last one ("Auto Merge"), which was only recently implemented. Personally, I think this is only appropriate for trivial pull requests which are obviously correct. For anything more complex, I would want to use the first approach, i.e.

  • adding the requester's repository as a new remote
  • fetching from that remote
  • trying the merge
  • testing carefully
  • pushing the result if you're happy

That means that the merged version is only public once you've tested it and decided to push. If you don't want to, you can just reset your master branch to its previous position.

As a matter of interest, it might be worth saying more about what happens if you do end up having to revert a regrettable merge, but still want to have the option to re-merge a later version of that branch. Although it might feel wrong, as I understand it the easiest way to deal with that situation is indeed to revert the revert. You can find more discussion of this issue in this post from the Pro Git blog and another discussion of the same problem by Linux Torvalds that might also be helpful.

Mark Longair
  • 446,582
  • 72
  • 411
  • 327
  • Hi Mark - I've read those last 2 articles before and am appreciative of you reminding me of them. I would love to implement your suggestion, except that none of our Organization members have their own fork on Github, they merely clone from our repo, as our boss wanted every dev to push their work to feature branches, just so we have a copy of their work. However, I might be able to setup all of the "remotes" on a single shared machine and have them push to that so that we could test. The big hurdle will be automating the deployment/review process. Thanks for the insight! – Chip Castle Nov 02 '11 at 14:26
  • @Chip Castle: No problem. In fact, it's even easier if they're just pushing to feature branches - you don't need to add a remote, just do `git fetch origin` and try merging from the right remote-tracking branch. – Mark Longair Nov 02 '11 at 16:02
  • Hmmm... our current process requires that a QA person manually reviews it after our CI passes. I need to configure Jenkins to "git pull --rebase origin master" to have the latest copy, then checkout the feature branch and "git rebase -i" to ensure everything is up to date before running the test suite. If that passes, I could have it auto-deploy to a test server where our QA reviews. Then if all of that passes, I could then accept the Pull Request, since that's the only action that would merge into master. That might help a lot. Let me know your thoughts on this. Thanks! – Chip Castle Nov 02 '11 at 21:28
1

I would suggest you guys take a different approach. Your workflow of reverting and reverting reverts seems very confusing to me. The actual problem you are trying to solve can be tackled differently.

I suggest you change your workflow to use two branches. One stable branch (master) and one development branch (develop). All work goes into the develop branch, or into separate topic branches. Pull requests are always filed against the develop branch, then merged into develop when approved.

master is initially branched off of develop. As soon as develop is in a stable state, you merge it into master. master is the current stable release.

This is loosely based on nvie's "A successful Git branching model".

igorw
  • 27,759
  • 5
  • 78
  • 90
  • I've read that article before and it's a great method to follow for sure, but it didn't seem to work well for our team. We already use feature branches and CI runs after one is merged into master (via a Github Pull Request), so having a develop branch, which we tried for a while, was simply more work to manage. Plus, even with our test suite, there were occasions when master became unstable, so it didn't really help our team that much. However, I'll keep this in mind going forward. Thanks again for your suggestion. – Chip Castle Nov 01 '11 at 18:12
1

If you get a branch-per-feature regiment going, you can rebuild a release candidate with what features you like. You will not need to "revert a merge":

further reading: https://plus.google.com/109096274754593704906/posts/R4qkeyRadLR

Please see the comments as well for additional insight. It works really well for us.

user229044
  • 232,980
  • 40
  • 330
  • 338
Adam Dymitruk
  • 124,556
  • 26
  • 146
  • 141