14 ms·
GitLab CEO here. I agree that GitFlow is needlessly complex and that there should be one main branch. The author advises to merge in feature branches by rebasin
by sytse 11y ago
GitLab CEO here. I agree that GitFlow is needlessly complex and that there should be one main branch. The author advises to merge in feature branches by rebasing them on master. I think that it is harmful to rewrite history. You will lose cherry-picks, references in issues and testing results (CI) of those commits if you give them a new identifier.
The power of git is the ability to work in parallel without getting in each-others way. No longer having a linear history is an acceptable consequence. In reality the history was never linear to begin with. It is better to have a messy but realistic history if you want to trace back what happend and who did and tested what at what time. I prefer my code to be clean and my history to be correct.
I prefer to start with GitHub flow (merging feature branches without rebasing) and described how to deal with different environments and release branches in GitLab Flow https://about.gitlab.com/2014/09/29/gitlab-flow/ https://about.gitlab.com/2014/09/29/gitlab-flow/
- brento 11y agoThere is nothing wrong with rebasing a feature branch imho. Feature branches should be considered ephemeral. But it probably depends on your team and project size.
- sytse 11y agoMy personal opinion is that it breaks history and CI tests for all the feature branches. But at GitLab we encountered customers that insisted on having a linear history after migrating from SVN. Therefore there is a function in the UI of GitLab EE to rebase a merge request when accepting the merge request. See http://doc.gitlab.com/ee/workflow/rebase_before_merge.html http://doc.gitlab.com/ee/workflow/rebase_before_merge.html
- solutionyogi 11y agoCould you expand on what do you mean by rebase breaking CI tests? I don't see how that's possible.
- tokenizerrr 11y agoA CI test result is tied to a particular commit hash. If you change this commit hash the CI test result will no longer be tied to that one commit.
- plorkyeran 11y agoThe results from previous CI runs are no longer connected to the rebased commits, and most people don't have CI set up to retest each commit individually after a rebase. I've never found this to be an actual problem in practice.
- tehbeard 11y agoSince a rebase changes the commit sha, I assume that's used as a reference for which version of the code the CI test was run against. That said, shouldn't CI be triggered after the rebase anyway to confirm all still works?
- solutionyogi 11y agoI see. But as you noted, one must do CI tests again after a rebase.
- sytse 11y agoIndeed you need to do them again. But you also have to rerun them after a merge anyway. The problem is that you can no longer see which commits that you merged where green before the merge. For example is very useful if the merge itself breaks the tests (uncommon but it can happen).
- masklinn 11y agoA big annoyance is UI: * most git UI don't provide for branch filtering or --left-only (which hides "accessory"/"temporary" merged branches unless explicitly required) * developers won't necessarily care for correct merge order, breaking "left-only" providing a mainline view The end result is, especially for largeish projects, merge-based workflows lead to completely unreadable logs.
- sytse 11y agoI'm not sure I completely understand. But it would be great if you can send small merge requests to GitLab to try to improve these things.
- solutionyogi 11y agoIt is better to have a messy but realistic history if you want to trace back what happend and who did and tested what at what time. I prefer my code to be clean and my history to be correct. ^^^ Couldn't agree more. However, I don't know why people want to avoid rebasing feature branches. Rebasing feature branches means that you only have to resolve the conflict once and have a clean history for your release branch. Granted, it works well in my team where only a single developer owns a given feature branch.
- aidos 11y agoI don't think that's true. If you have a feature branch with a number of changes in the same place, rebasing on to a branch that also changes in the same spot means you need to fix all of the related commits during the rebase. If it's a big feature that could end up being a huge task. I may be wrong about that as I'm no git guru.
- solutionyogi 11y agoTrue. You do have to fix all the true conflicts. But you have to do it only once. If you merge your master, you have to resolve conflicts every time you merge from the master. git rerere could be potentially useful here. Even more importantly rebase allows you to resolve conflict at individual commit level. During conflict, you can see the exact commit that is causing it, making it much easier to resolve it. See: http://jeffkreeftmeijer.com/2010/the-magical-and-not-harmful-rebase/ http://jeffkreeftmeijer.com/2010/the-magical-and-not-harmful...
- Mithaldu 11y agoLooking at the recent history i can see how you'd come to like it. You seem to mostly be doing merges or documentation changes, which probably means you don't have to do a lot of history spelunking to fix bugs caused months ago. Are you sure your developers feel the same as you do? Are you sure they're willing to be open enough to you about their misgivings?
- sytse 11y agoI think the majority of people on our team don't like rebasing because it makes spelunking harder in some cases. But there are certainly people that preferred having some commits rebased so it was easier to revert them (reverting a merge is possible but harder). Although I'm not an active developer myself I think my dislike of rebasing everything is shared.
- Mithaldu 11y ago> I think the majority of people on our team don't like rebasing because it makes spelunking harder in some cases. Would you elaborate on that please? I don't know what your situation is like that would cause rebasing to make spelunking harder. > reverting a merge is possible but harder Funny, someone else further down claimed reverting merges is easier. :)
- sytse 11y agoBecause you rewrote history it is harder to see what was written, tested by hand and tested with CI at what time.
- Mithaldu 11y agoAnother social issue. In the projects i work with, it is commonly agreed upon and known that of course after a rebase every commit needs to be retested and reverified with CI. It is a bit of extra work, but as i mentioned in another commit: We do that extra bit of work now, to avoid having to analyze the cross-talk of a bunch of branch merges at a later point; mainly because that bit of later work often turns out to be considerably bigger. (Technical debt is a talking point here.) Of course, this might be unviable when under unusual time pressures.
- radicalbyte 11y agoSytse, do you mind if I ask a slightly OT question? How does GitLab store the code-review data? Is it stored in the (or a) git repo? Is the feature compatible with rebasing feature branches before merge? Also, pricing: I only just noticed that your pricing was per YEAR, not per MONTH. Most boostrap-pricing-page software is priced monthly and the user/year text is lowlighted. This has to be costing you sales.
- sytse 11y agoYou're very welcome. When you accept a merge request the title, description and a link to the merge request are stored as the commit message. For example see https://gitlab.com/gitlab-org/gitlab-ce/commit/6c0db42951d65eeb231046d25302afbd42a43a69 https://gitlab.com/gitlab-org/gitlab-ce/commit/6c0db42951d65... This allows you to see any other things that were discussed. Hopefully any line comments were resolved with a commit (thus documenting them) or were based on a misunderstanding. Thanks for the pricing tip, we'll fix it https://gitlab.com/gitlab-com/www-gitlab-com/issues/348 https://gitlab.com/gitlab-com/www-gitlab-com/issues/348
- mikeash 11y agoThe obsession of git users with rewriting history has always puzzled me. I like that the feature exists, because it is very occasionally useful, but it's one of those things you should almost never use. The whole point of history is to have a record of what happened. If you're going around and changing it, then you no longer have a record of what happened, but a record of what you kind of wish had actually happened. How are you going to find out when a bug was introduced, or see the context in which a particular bit of code was written, when you may have erased what actually happened and replaced it with a whitewashed version? What is the point of having commits in your repository which represent a state that the code was never actually in? It always feels to me like people just being image-conscious. Some programmers really want to come across as careful, conscientious, thoughtful programmers, but can't actually accomplish it, so instead they do the usual mess, try to clean it up, then go back and make it look like the code was always clean. It doesn't actually help anything, it just makes them look better. The stuff about nonlinear history being harder to read is just rationalization.
- sytse 11y agoI totally agree with you, I don't get it either. My only explanation is that as a programmer you are trained to write clean and understandable code. I also try to apply this to my commit messages (with varying results). But rewriting history to make everything look clean and simple is the wrong this to do. The messier your history is the more likely you'll need to retrace your steps (and CI results) at some point. It is mostly people coming from SVN and only running CI on the trunk branch that favor the rewriting approach. It might be hard to let go.
- Mithaldu 11y ago> It is mostly people coming from SVN and only running CI on the trunk branch that favor the rewriting approach. It might be hard to let go. You're being ridiculously prejudiced and jumping to conclusions AND throwing out judgements on things that by your own admission ("I don't get it either.") you do not understand. Please realize that the correct response in such a case is not to double down, but to engage in a dialogue so you may reach understanding of which factors you're unaware of or they're unaware of, that create the difference in stance. (And no, you can't expect the other side to initiate the dialogue. The way you are talking you present an image of someone singularly uninterested in dialogue, even if that may be unintentional.)
- ChrisMeek 11y agoI'm starting to think that part of the problem we are facing at the moment is that feature branching itself can be exceptionally harmful. I see statements like "The power of git is the ability to work in parallel without getting in each-others way" and get really worried about what people are trying to achieve. I want my team's code to be continuously integrated so that problems are identified early, not at some arbitrary point down the line when two features are finished but conflict with each other when both are merged. We seem to be reversing all the good work the continuous integration movement gave us; constant integration makes integration issues smaller and easier to fix. I personally prefer to use toggling and techniques like branch by abstraction to enable a single mainline approach. Martin Fowler has a very good article on it here http://martinfowler.com/bliki/FeatureBranch.html http://martinfowler.com/bliki/FeatureBranch.html
- platz 11y agoAgree, avoid large merges at all costs. the longer a feature branch exists the more needless cost it incurs when trying to re-integrate it with the merge.
- mcv 11y agoAbsolutely. Though you can mitigate that by merging develop into the feature branch often. It makes the merge back pretty painless. Of course if you've got two long-living, contradictory feature branches, merging develop is not going to be enough. I guess you still need some communication in the team. But it's also important to keep your branches as short as you can. If you got a really big feature, try splitting it up.
- jes5199 11y agoThose are good techniques and I advocate them strongly. Even so, I see value in very short-lived branches for code review. Ideally a branch exists for about four hours before it is merged to master.
- sytse 11y agoThe idea is that you split features up in smaller parts so that you merge not more than a handful of days of work at a time.
- DougWebb 11y agoIf you're using CI testing results and tying them to particular commits, you end up with the same problem whether you merge or rebase. If you test a commit and it passes, and then merge that commit into master, the merge may have changed the code that the commit modified, or something that the commit's code depended on. The green flag you had on the commit is no longer valid because the commit is in a new context now and may not pass. If you rebase the commit onto master, you're explicitly changing the context of the commit. Yes, you get a different SHA and you're not linked to the original CI result anymore, but that CI result wasn't valid anymore anyway. This is exactly the same situation that the merge put you into, but without the false assurance of the green flag on the original test result. As many others have noted, rebasing is only recommended on private branches to prepare them for sharing on a public branch. If you're running CI it's probably only on the public branches, so rebasing wouldn't affect that. But if you're running CI on your private branch too, then you're going to want to run it after rebasing onto the public branch and before merging into the public branch. That gives you assurance that your code works on the public branch before you share it. Again, if you're using a merge-based workflow you'd have to do the same testing regardless of your earlier test results.
- notduncansmith 11y agoMy intuition of how CI should work is that it tests what the new master would look like were that commit [rebased|merged] into the current master, for that exact reason. Is that not how CI systems tend to work? What you've described above does seem awfully silly.
- jamesmiller5 11y agoThis is exactly how I setup our teams workflow. Private commits are pushed into gerrit run against the CI suite. 1. If reviews + CI tests go well we fast forward merge onto master. 2. If the commit's parent isn't the latest commit on master, it is automatically rebased and the CI suite is kicked off again. 3. Upon successful fast forward merge into master, all in-flight reviews are automatically rebased on master's new head and CI's kicked off again. 4. Any open commit can become the top of master without worry it will break the build. For our team of ~10 this works exceptionally well with master not being a broken due to our code in the last ~6 months. (edit: formatting)
- dasil003 11y ago> * In reality the history was never linear to begin with. It is better to have a messy but realistic history if you want to trace back what happend and who did and tested what at what time. I prefer my code to be clean and my history to be correct.* I disagree very very strongly with this. I wrote about this years ago at http://www.darwinweb.net/articles/the-case-for-git-rebase http://www.darwinweb.net/articles/the-case-for-git-rebase, but it's due to for an update to clarify my thinking. Basically my beef is the idea that never rebasing is "true" history, and rebasing gives you "corrupt" or "whitewashed" history. In fact, the only thing you have weeks, months, years after pushing code is a logical history. It's not as if git automatically records every keystroke or every thought in someone's head—that would be an overwhelming amount of information and difficult to utilize anyway—instead it's all based on human decisions of what to commit and when. Rebasing doesn't "destroy" history, it's just a decision of where a commit goes that is distinct from the time it was first written, but in fact you lose almost no information—the original date is still there, and from that you can infer more or less where it originally branched from. "But," you say, "surely complete retention of history is preferable to almost-complete retention?". Well, sure, all else being equal I would agree with that. But here's the crux of the issue: merging also loses information. What happens when you merge two conflicting commits is that bugs are swallowed up in merge commits rather than being pinned down to one changeset. This is true whether it is a literal merge conflict that git detects, or a silent logic error that no one discovers until the program blows up. With two branches merging that are logically incompatible, whose responsibility is it to fix their branch? Well, whoever merges last of course, and where does that fix happen under a never-rebase policy? In that single monstrous merge commit that can not be reasoned about or bisected. But if you always rebase before merging to master, then the second integrator has to go back and correct each commit in the context of the new branch. In essence, they have to fix their work for the current state of the world, as if they had written it today. In this way each tweak is made where it is visible and bisectable instead of squashed into an intractable merge commit. I get that there is some inconvenience around rebasing coordination and tooling integration (although GitHub PRs handle it pretty well), but the idea that the unadulterated original history has significant value is a straw man. If the branch as written was incompatible at the point it got merged, there is no value in retaining the history of that branch in an incompatible state because you won't be able to use it anyway. In extreme cases you might decide the entire branch is useless and just pitch it away entirely, and certainly no one is arguing to save history that doesn't make it onto master right?
- mrbobdobbs 11y agoAt our offices we use Pull Requests and Rebase as a combined work flow to get mostly linear history. Before issuing a PR we get the latest master, rebase our branch onto that so that all commits in our branch are approximately at the same time stamp and then issue a PR. This creates a nicely linear history for the most part. The only evil is a willingness to force push the updated history over our branches before the PR goes up. But no one shares branches usually. Or the collaborators on a branch are few and they agree when to rewrite history.