I agree this is a better way to look at pull requests, but I'm surprised it's as much of a problem as the post implies. For us, whenever we do a pull request, we always rebase against the latest master first. That can be a cat and mouse game, but it usually isn't. Our repos don't change that rapidly.
The problem is, many pull requests take very long to get merged (because of the size of the PR, the review process, etc.), and by the time that they are ready to be merged, the target branch could very well have changed.
This is something that our team has relegated to a product development problem not a software development problem. As a product manager/technical lead you have to realize it's your job to make it as easy as possible for developers to do their job. When you set them up for HUGE merge conflicts - that only makes life harder. If you're really good about constructing discrete, small stories, you can avoid SO many pull request headaches. If you don't allow master to get miles ahead by merging small changes frequently, this is a non-issue. It's hard to do though and requires changing the mindset of your team.
Great point. Almost all of the problems I've seen with pull requests and branching workflows stem from the size of the change. If you can avoid long-lived branches / giant diffs, most integration problems just disappear. It's not always possible to avoid them, but bearing it in mind when you're constructing stories and planning your backlog helps a lot.
This is more of a problem in open-source projects where occasionally someone would drop by a project and submit a PR without any prior planning, and more often than not disappear for a while before coming back to address your comments/review. It is less of a problem in closely-tied teams.
Our solution to that is to always rebase & push right before you run through ci & merge. As well as updating the PR diff to be against the master it was actually merged against, this also has the advantage of making your git history a lot more linear[0], which has various other upsides (e.g. makes it a lot more sane when using fugitive or something to navigate backwards & forwards through the history of a file).
Talking to a lot of people at conferences and similar events, rebasing against the target branch before merging is pretty uncommon. Few people think a clean history is that important.
Personally I think, rebase + auto-squash/auto-fixit makes the history a lot easier when it's time to look back. It just happens so rarely I wonder if it's really worth the effort I expend on it.
I don't know what it is, but when I try to rebase master into the feature branch, my PR diff ends up littered with commits that aren't part of the PR. Then my reviewers have to wade through a bunch of irrelevant crap to see my changes. I thought the whole point of rebasing was so that wouldn't happen.
Anyway, now I just don't do it anymore. Git is a pain in the ass.
If you rebased master into the feature branch, then what you said makes sense. I realize it may look like semantics, but you rebase your feature branch onto the master branch. This means something completely different.
Huh. If I rebase master into the feature branch, isn't that supposed to move the point at which the feature branched off of master from where it was, to the HEAD of master?
About rebasing the feature branch onto master, my team doesn't do that, we squash the feature branch commits into one commit when merging to master.
(I don't use anything besides git, but that doesn't mean I can't hate it)
No, if you rebase master into the feature branch, that will move all of the new commits to master into the feature branch.
Now, it could be that I am just being a stickler for phrasing here. So, to clarify, if you are on the branch and run 'git rebase master', that is not rebasing master into the feature branch. That is rebasing the feature branch onto master.
My phraseology was wrong, apologies. Yes that is what I was doing. You can probably tell that my default attitude towards git is one of confusion and frustration, this is no different
No worries. I can not claim that it is a simple problem to just immediately understand. Worse, I am not good enough in my understanding, to explain in a message forum. :(
I can say that it is easy, once you understand it more. It will take some time.
Countering that point, though; if you have a codebase that is rapidly changing at all times... there really isn't anything git can do to help.
Well, the problem for me is what I perceive to be the discrepancy between how rebase is supposed to work, and what it actually does when I use it. I would like to think I understand it but I guess you could argue I don't. At any rate I'm not really learning about it, I just don't do it anymore because I kept getting burnt by it.
My teammates have suggested to simply merge master into the branch so I do that now. It adds a commit to the branch, but Github is smart enough not to litter up the PR diff with the merge commit.
Our codebase doesn't change that much, every commit to master has to go through a PR and get approved. So fortunately we don't have to contend with that.
Only thing I can say is that rebase is exactly the same as reseting to the merge base, then cherry picking each individual commit that was lost in the reset. This means if you did 10 commits locally, it may stop 10 times to have you fix things. Versus a merge, which just does a single commit.
That said, I will also say that any worries about having merge commits in the history should largely be overcome. They actually provide useful information and are easy to ignore if you want.
But again, the age-old response to this flow is that it creates an untrue version history. This is great if cleanliness is your exclusive priority, but if you find that you need to actually try to jump into the mind of the person who made the commit (especially if it's someone you've never met and with whose habits you aren't familiar), it's impossible to actually follow along and recreate what really happened.
The rebase strategy and merge strategy both have their places - neither is fit for "whenever we do a pull request" for all people everywhere.
I don't follow because the rebase is done by the person who wrote the code and is proposing the pull request. They rewrite their local history, but that doesn't seem to relevant. They still have to get their code to work with the latest master and remain in control of it. When the PR goes through, it can be a standard merge to maintain history.
True, but while that person probably tested each original commit as they wrote it (at least to the extent the application compiled and didn't completely blow up), they probably didn't do the same for each rebased commit every time they updated to master. If the resulting history turns out to be broken, it isn't the end of the world, but makes bisecting harder.
I think rebasing is a good idea anyway, but it's a tradeoff.
There's no perfect solution and all of this still requires a fair amount of vigilance to get it right. But I would still argue that if you are proposing a PR, you've done the work to verify your changes work, tests pass, etc. If you later need to rebase your PR, you still should run tests, smoke test, etc.
Small issues can slip through, but on teams I've worked on, submitting a PR that is truly broken is really bad form. Sometimes that does mean submitting PRs is tedious, but that's just how it goes sometimes. Thankfully, it's not that often in my experience.
