5 ms·

It's not 'editing' the raw history files, but you're still presenting a false history to your coworkers. To me there are essentially two kinds of rebases: - Su

It's not 'editing' the raw history files, but you're still presenting a false history to your coworkers. To me there are essentially two kinds of rebases: - Summarizing history: squashing "implemented subfeature A.A" and "implemented subfeature A.B" into "implemented feature A" - Rewriting history: moving commits around, changing the base commit, and so on In my opinion summarizing history is acceptable, you're making a creative decision that certain information will not be useful in the review/when trying to understand the code in the future. Rewriting, on the other hand, is essentially lying. You're creating repository states that never existed, and which you have never tested. In the worst case, consider the following history: * F: (master) Merge branch 'component2' |\ | * E: (component2) Fixed component 2's integration with 1 | * D: (component2) Merge branch 'master' into component2 | |\ | |/ |/| * | C: (master) Refactored component 1's API | * B: (component2) Implemented component 2 that depends on 1 |/ * A: (master) Base Yes, it could probably be completely linearized, but that would be a horrible idea. Commit B will leave the repository in a completely nonsensical state. Sure, you could squash in E to mitigate it (since, luckily, nothing else happened in component 2 in the meantime), but then you're still stuck explaining what will likely look like a bunch of really weird design decisions compared to if you had designed against component 1's new API immediately. Historical context matters. If in doubt, don't rebase. Never `git pull --rebase` blindly.

gwright 7y ago
It isn't lying. Advocates of rebase are always talking about a work flow where you are curating a set of proposed changes before merging into your "public" branches (.e.g, development or master). No one is advocating that you use rebase on your public branches or basically any branch that has been "published". We are talking about feature branches or spikes or branches that exist just on one developer's machine.
- Nullabillity 7y ago
  You're lying about the path that you took to get to that point, and you're creating a lot of (public) nonsense commits on the way there (unless you're very careful, and/or overly squash-happy). Whether you've published the true history earlier is irrelevant to that discussion.
  - gwright 7y ago
    Let me try again. I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all. If I make three commits and then realize that I should have included something in the first commit, I use rebase to create a new sequence of three commits that has the corrected version of the first commit. I haven't shared those commits with anyone, this is just work that I've done locally. Are you seriously advocating that creating a pull request with: (A, B, C, A-fixup) is better than using rebase and then creating a pull request with: (better-A, B, C)? You think that second case is "lying" because I didn't show the intermediate step that included the mistake?
    - Nullabillity 7y ago
      Yes, it is lying. You can mitigate most of the damage if it is convincing enough (for example, go through B' and C' and make sure everything still makes sense at each point), but realistically nobody is going to do that, because it's pretty inefficient way to spend your time. And even then, you're still removing context (unless you're just fixing a typo). > I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all. That was clear from the start. But the fact that X breaks Y doesn't imply that Y is a good idea when X doesn't apply.
      - gwright 7y ago
        I really don't understand why you are choosing to use the word "lying". Us mere humans make mistakes all the time. Typos, omissions, false starts, and so on. What is the value of throwing that raw set of events at a reviewer or complicating the understanding of the changes when viewed in retrospect from the future? What is the reason you call curating the work into a more polished form "lying"? Why do you think the time spent being intentional about changes isn't valuable when compared to the time spent by a reviewer (or your future self) to sort through the flotsam and jetsam of your intermediate work?
        7y ago
gwright 7y ago
Or you could just rebase component2 when you learn that component1 has been updated on master: * F: (master) Merge branch 'component2' |\ | * E: (component2) Implemented component 2 (now with updated component1) |/ * C: (master) Refactored component 1's API | * A: (master) Base So we've lost B and D from your example, but who cares about those commits?
- Nullabillity 7y ago
  That's easy in the trivial example where nothing else happened in the meantime.
palunon 7y ago
> and which you have never tested. That's on you if you don't test every commit. I don't care if you had failing tests (or even build) when you were writing your feature. I care that every one of your patches (and thus commit) does one logical thing, and that tests passes (making bisect useful). You want your PR commit history to tell a coherent story. No one care if a writer had 15 bad draft of their story before publishing, and the same apply here.
fulafel 7y ago
Yes, good explanation of the problem. And the cherry on top is that you can't easily tell later if you are looking at rewritten history. So if the above kind of rewriting might have happened in your project, you will essentially not be able to trust git history anymore as a record of engineering decisions.
dahart 7y ago
> you’re still presenting a false history to your coworkers [...] essentially lying This has been a common misunderstanding of git in the past, but thankfully is fading now. I was hoping it wouldn’t come back to haunt this thread. I don’t know where the extreme and hyperbolic idea of using git the way it was designed is “lying” and creating “false history” first came from, you aren’t the first person to suggest it, but it’s neither correct nor helpful to use that kind of language. This theoretical philosophical ideal that there’s only one true history is trading away things git was specifically created to do, as well as the practicalities of real world software development, in favor of a strange unrealistic and abstract notion that once git commit has been used the commit should never be touched again. Everyone knows and agrees that rearranging already published commits is a bad idea. Not because it’s “lying”, but because it causes problems, costs other people time, and can even inflict irreconcilable merge conflicts on their work. Cleaning up your own commits before you push using interactive rebase is not just a good idea, it’s the way git was designed, it’s what Linus does, and it’s kind to your team. This includes reordering commits and pulling with rebase. > Historical context matters. If in doubt, don’t rebase. Never `git pull -- rebase` blindly. Maybe you could back up your assertion with some examples of why it always matters, and why that justifies using words like ‘never’? Your rhetoric is ignoring the real-world fact that on a large team, the majority of commits at any given time are orthogonal to each other, and that the parent commit you end up with is completely arbitrary. Not only do I use pull -- rebase, I always git config --global pull.rebase true, and I frequently recommend others do the same. Having merge commits in master every single time someone checks in is incredibly noisy and it inflicts friction on the entire team to force everyone to read the noisy log. I’ve always worked on teams that decided to take the more practical approach of one-off commits should not have a merge, regardless of when they happen, to keep history cleaner, and feature branches with more than a couple of commits or by more than one person should have a merge commit, to keep the master branch from having broken commits or unfinished features and so it’s always bisectable.

gwright 7y ago

It isn't lying. Advocates of rebase are always talking about a work flow where you are curating a set of proposed changes before merging into your "public" branches (.e.g, development or master). No one is advocating that you use rebase on your public branches or basically any branch that has been "published". We are talking about feature branches or spikes or branches that exist just on one developer's machine.

Nullabillity 7y ago
You're lying about the path that you took to get to that point, and you're creating a lot of (public) nonsense commits on the way there (unless you're very careful, and/or overly squash-happy). Whether you've published the true history earlier is irrelevant to that discussion.
- gwright 7y ago
  Let me try again. I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all. If I make three commits and then realize that I should have included something in the first commit, I use rebase to create a new sequence of three commits that has the corrected version of the first commit. I haven't shared those commits with anyone, this is just work that I've done locally. Are you seriously advocating that creating a pull request with: (A, B, C, A-fixup) is better than using rebase and then creating a pull request with: (better-A, B, C)? You think that second case is "lying" because I didn't show the intermediate step that included the mistake?
  - Nullabillity 7y ago
    Yes, it is lying. You can mitigate most of the damage if it is convincing enough (for example, go through B' and C' and make sure everything still makes sense at each point), but realistically nobody is going to do that, because it's pretty inefficient way to spend your time. And even then, you're still removing context (unless you're just fixing a typo). > I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all. That was clear from the start. But the fact that X breaks Y doesn't imply that Y is a good idea when X doesn't apply.
    - gwright 7y ago
      I really don't understand why you are choosing to use the word "lying". Us mere humans make mistakes all the time. Typos, omissions, false starts, and so on. What is the value of throwing that raw set of events at a reviewer or complicating the understanding of the changes when viewed in retrospect from the future? What is the reason you call curating the work into a more polished form "lying"? Why do you think the time spent being intentional about changes isn't valuable when compared to the time spent by a reviewer (or your future self) to sort through the flotsam and jetsam of your intermediate work?
      - 7y ago

gwright 7y ago

Or you could just rebase component2 when you learn that component1 has been updated on master: * F: (master) Merge branch 'component2' |\ | * E: (component2) Implemented component 2 (now with updated component1) |/ * C: (master) Refactored component 1's API | * A: (master) Base So we've lost B and D from your example, but who cares about those commits?

Nullabillity 7y ago
That's easy in the trivial example where nothing else happened in the meantime.

palunon 7y ago

> and which you have never tested. That's on you if you don't test every commit. I don't care if you had failing tests (or even build) when you were writing your feature. I care that every one of your patches (and thus commit) does one logical thing, and that tests passes (making bisect useful). You want your PR commit history to tell a coherent story. No one care if a writer had 15 bad draft of their story before publishing, and the same apply here.

fulafel 7y ago

Yes, good explanation of the problem. And the cherry on top is that you can't easily tell later if you are looking at rewritten history. So if the above kind of rewriting might have happened in your project, you will essentially not be able to trust git history anymore as a record of engineering decisions.

dahart 7y ago

> you’re still presenting a false history to your coworkers [...] essentially lying This has been a common misunderstanding of git in the past, but thankfully is fading now. I was hoping it wouldn’t come back to haunt this thread. I don’t know where the extreme and hyperbolic idea of using git the way it was designed is “lying” and creating “false history” first came from, you aren’t the first person to suggest it, but it’s neither correct nor helpful to use that kind of language. This theoretical philosophical ideal that there’s only one true history is trading away things git was specifically created to do, as well as the practicalities of real world software development, in favor of a strange unrealistic and abstract notion that once git commit has been used the commit should never be touched again. Everyone knows and agrees that rearranging already published commits is a bad idea. Not because it’s “lying”, but because it causes problems, costs other people time, and can even inflict irreconcilable merge conflicts on their work. Cleaning up your own commits before you push using interactive rebase is not just a good idea, it’s the way git was designed, it’s what Linus does, and it’s kind to your team. This includes reordering commits and pulling with rebase. > Historical context matters. If in doubt, don’t rebase. Never `git pull -- rebase` blindly. Maybe you could back up your assertion with some examples of why it always matters, and why that justifies using words like ‘never’? Your rhetoric is ignoring the real-world fact that on a large team, the majority of commits at any given time are orthogonal to each other, and that the parent commit you end up with is completely arbitrary. Not only do I use pull -- rebase, I always git config --global pull.rebase true, and I frequently recommend others do the same. Having merge commits in master every single time someone checks in is incredibly noisy and it inflicts friction on the entire team to force everyone to read the noisy log. I’ve always worked on teams that decided to take the more practical approach of one-off commits should not have a merge, regardless of when they happen, to keep history cleaner, and feature branches with more than a couple of commits or by more than one person should have a merge commit, to keep the master branch from having broken commits or unfinished features and so it’s always bisectable.