4 ms·

> The head commit on master is the same, but the state of previous commits from your branch becomes different and untested, and quite possibly broken. Try to b

by patrec 6y ago

> The head commit on master is the same, but the state of previous commits from your branch becomes different and untested, and quite possibly broken. Try to break it down, and you will see that the scenario you're positing doesn't make much sense and your concerns are unfounded: Let's posit the following: 1. We have a feature branch B, with carefully crafted and individually tested commits. 2. We have a minimally working CI setup (which means we always test the code state of the new head of master before landing stuff in master "for real"). On that assumption one of two things can happen when you try to land B whether via rebase or merge: a) CI fails, because changes in master in the meantime make it so your branch now triggers a test failure or merge conflict whereas before it didn't. We can ignore this, scenario, because nothing lands till b) CI passes. So we know the HEAD commit on master is fine. But, you say, whilst I know the individual commits on B are also good (I tested them all) and the merge commit is good (CI!), with rebase I only know the head commit works -- what if the rebasing broke some intermediate commits? But for that to happen, all the following need to be true: 1. You don't didn't use rebase -x (why not, if you carefully test all individual commits on the feature branch?) 2. Your individual commits all work correctly on the feature branch and would pass CI. 3. The sum of your individual commits plus master works correctly and definitely does pass CI. 4. Because of changes in master branch since the common ancestor of present master and B, a) at least one of the commits of B no longer would pass CI when tested, although it does on the branch (not an unlikely scenario so far) AND b) a latter commit on the feature branch miraculously fixes this incompatibility (because we know the HEAD passes). This is not theoretically impossible, but exceedingly unlikely. And, you can easily guard against it happening as described above.

lmm 6y ago
> a) CI fails, because changes in master in the meantime make it so your branch now triggers a test failure or merge conflict whereas before it didn't. We can ignore this, scenario, because nothing lands till No we can't, because what do people do in this scenario? They make a new commit that fixes the tests, but leaves a long chain of previous commits broken. (Also in a big enough codebase with enough collaborators it becomes impractical to require CI to pass against the absolute tip of your shared development branch before merging, because in the time it takes to run CI there will always have been more changes landing in that shared branch). > 1. You don't didn't use rebase -x (why not, if you carefully test all individual commits on the feature branch?) I've never known anyone to actually use -x. Maybe a very dedicated individual could, but there's no way you'd be able to enforce it across a team (unless that team is in the habit of making feature branches that are just a handful of big commits - but in that case you might as well just squash-merge). Pulling changes from master and resolving any conflicts is already a frustrating interruption; making it take 20x longer is a non-starter. People don't carefully test all individual commits, but that's ok. Even the most casual testing will catch when a commit simply doesn't compile, and introducing a compilation error in manual code changes is much less common than introducing a compilation error when pulling changes from master. And in the worst case, an isolated non-compiling commit in history is not too much of a problem to skip when bisecting. The problem only comes when you have a long chain of non-compiling commits - which is exactly what rebasing tends to produce.
- dragonwriter 6y ago
  > Maybe a very dedicated individual could, but there's no way you'd be able to enforce it across a team Adopt standard workflow tooling that enforces it, like the gitflow tool but for your actual team workflow.
  - lmm 6y ago
    This is vaporware; meanwhile a merge-based workflow is working fine in many organisations every day.
- patrec 6y ago
  > No we can't, because what do people do in this scenario? They make a new commit that fixes the tests, but leaves a long chain of previous commits broken. Same thing will happen with merges -- one can try to argue (as you have done) that with merges it will be less bad, because you may on average end up with fewer broken commits -- and I don't think that is implausible. But that is basically just saying your existing way to bisect is not fully reliable and you fear using rebase would make it quantitatively noticeably worse. What I am saying is it can and should be fully reliable, and this is not theoretical either. I enjoy discussing different workflows with people with a different outlook and experience and who have put a decent amount of thought into it, as you clearly have. But I still have to note that there is a certain irony here: you have something which by your own admission is not completely reliable; i.e. not all your merges in master have passed CI and you will have some (hopefully small) proportion of commits that you'll need to manually bisect skip because they are bad for one reason or other and you won't know in advance. You are adamant that rebase-based workflows are bad (partly) because you fear that they will greatly exacerbate such problems and you don't seem to think it's practically possible to completely avoid them to start with. But I am arguing from plenty of real-life experience with a workflow, which is incidentally rebase-based (but the same guarantees would hold just as well if I moved it to a purely merge based approach) where I a) know that every "merge" to master has been fully tested before it lands b) can reliably bisect over ~years of commits with a simple git alias, without having to ever manually bisect skip bad commits[+]. And by construction, not because I hope that devs will generally have tested their commits sufficiently manually or are diligent about fixing merge conflicts in a way that does not result in intermittent broken commits. So whilst several of your other reasons are perfectly valid (long-lived branches), you can maybe see why this one is a bit amusing to me. > (Also in a big enough codebase with enough collaborators it becomes impractical to require CI to pass against the absolute tip of your shared development branch before merging, because in the time it takes to run CI there will always have been more changes landing in that shared branch). It's not impractical at all, in fact if your company pays me for it I'm more than happy to set it up for you :) Alternatively shoot me an email and I'll be happy to explain the gist of it (including how to completely avoid the problem you mention above). [+] Of course assuming nothing was broken at the meta level, e.g. no one accidentally temporarily misconfigured CI, so some stuff that should have been didn't get tested.

by patrec 6y ago

lmm 6y ago

> a) CI fails, because changes in master in the meantime make it so your branch now triggers a test failure or merge conflict whereas before it didn't. We can ignore this, scenario, because nothing lands till No we can't, because what do people do in this scenario? They make a new commit that fixes the tests, but leaves a long chain of previous commits broken. (Also in a big enough codebase with enough collaborators it becomes impractical to require CI to pass against the absolute tip of your shared development branch before merging, because in the time it takes to run CI there will always have been more changes landing in that shared branch). > 1. You don't didn't use rebase -x (why not, if you carefully test all individual commits on the feature branch?) I've never known anyone to actually use -x. Maybe a very dedicated individual could, but there's no way you'd be able to enforce it across a team (unless that team is in the habit of making feature branches that are just a handful of big commits - but in that case you might as well just squash-merge). Pulling changes from master and resolving any conflicts is already a frustrating interruption; making it take 20x longer is a non-starter. People don't carefully test all individual commits, but that's ok. Even the most casual testing will catch when a commit simply doesn't compile, and introducing a compilation error in manual code changes is much less common than introducing a compilation error when pulling changes from master. And in the worst case, an isolated non-compiling commit in history is not too much of a problem to skip when bisecting. The problem only comes when you have a long chain of non-compiling commits - which is exactly what rebasing tends to produce.

dragonwriter 6y ago
> Maybe a very dedicated individual could, but there's no way you'd be able to enforce it across a team Adopt standard workflow tooling that enforces it, like the gitflow tool but for your actual team workflow.
- lmm 6y ago
  This is vaporware; meanwhile a merge-based workflow is working fine in many organisations every day.
patrec 6y ago
> No we can't, because what do people do in this scenario? They make a new commit that fixes the tests, but leaves a long chain of previous commits broken. Same thing will happen with merges -- one can try to argue (as you have done) that with merges it will be less bad, because you may on average end up with fewer broken commits -- and I don't think that is implausible. But that is basically just saying your existing way to bisect is not fully reliable and you fear using rebase would make it quantitatively noticeably worse. What I am saying is it can and should be fully reliable, and this is not theoretical either. I enjoy discussing different workflows with people with a different outlook and experience and who have put a decent amount of thought into it, as you clearly have. But I still have to note that there is a certain irony here: you have something which by your own admission is not completely reliable; i.e. not all your merges in master have passed CI and you will have some (hopefully small) proportion of commits that you'll need to manually bisect skip because they are bad for one reason or other and you won't know in advance. You are adamant that rebase-based workflows are bad (partly) because you fear that they will greatly exacerbate such problems and you don't seem to think it's practically possible to completely avoid them to start with. But I am arguing from plenty of real-life experience with a workflow, which is incidentally rebase-based (but the same guarantees would hold just as well if I moved it to a purely merge based approach) where I a) know that every "merge" to master has been fully tested before it lands b) can reliably bisect over ~years of commits with a simple git alias, without having to ever manually bisect skip bad commits[+]. And by construction, not because I hope that devs will generally have tested their commits sufficiently manually or are diligent about fixing merge conflicts in a way that does not result in intermittent broken commits. So whilst several of your other reasons are perfectly valid (long-lived branches), you can maybe see why this one is a bit amusing to me. > (Also in a big enough codebase with enough collaborators it becomes impractical to require CI to pass against the absolute tip of your shared development branch before merging, because in the time it takes to run CI there will always have been more changes landing in that shared branch). It's not impractical at all, in fact if your company pays me for it I'm more than happy to set it up for you :) Alternatively shoot me an email and I'll be happy to explain the gist of it (including how to completely avoid the problem you mention above). [+] Of course assuming nothing was broken at the meta level, e.g. no one accidentally temporarily misconfigured CI, so some stuff that should have been didn't get tested.