5 ms·

The point of the parent comment is exactly that you should clean up the history before merging to a public branch, so that you can use bisect, even if so far yo

The point of the parent comment is exactly that you should clean up the history before merging to a public branch, so that you can use bisect, even if so far you had wip wip doh wip as the commit messages. The way to get there is to have a mix of proper and wip commits.

WorldMaker 3y ago
If merge points are your "known good" points anyway you can just use the powers of the git dag and `git bisect --first-parent` in your main branch to just bisect the merge points. There's no need for rebase/squash and you still get useful git bisect results.
- bonzini 3y ago
  All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.
  - WorldMaker 3y ago
    Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in development work-in-progress).
    - bonzini 3y ago
      I don't do full CI for every commit but I do run the relevant unit tests (or all of them depending on the change and the project) and ensure that they pass.
    - dkubb 3y ago
      I think the point the GP message is making is that, prior to review/merge you extract atomic commits from your WIP that tell a clear, concise story of how the change was made. The reviewer has less built up context so by chunking it like this they can step through each commit one at a time. IMHO the expectation is that each commit would 100% pass CI, so if you decided to extract some commits and merge that early you can. This is especially useful when a 6 commit PR is reviewed, and the first 3 commits are fine but there is more feedback on the last three. The reviewer can split the first 3 good ones out, get them merged and whittle down the PR to the remaining three. The subsequent follow up will be less. IME team velocity goes up with this too, and it encourages small and easy to review commits like a Remove to be extracted and merged early. Since PRs are always as large or larger than commits, I would much rather have a specific commit flagged than have to wade through the whole PR diff. If the PR is not familiar to me, I want to increase my effectiveness narrowing down the cause, so I can fix it faster.
watwut 3y ago
Frankly, people lately spend more time managing commit history then using it. Like, commit history is useful once in a year little bit, maybe, but we spend absurd amount of time trying to make it look nice.
- gwright 3y ago
  Not my experience, nor my team's experience over almost 10 years of using this approach.
  - Chris_Newton 3y ago
    I’m firmly in your camp on this one, but I’ve noticed that advocating a tidy history gets a lot of push-back online. I think there is an element of self-fulfilling prophecy here. If a team habitually leaves a messy history behind, that history is rarely going to be useful, so naturally the team has low expectations and sees little value in doing anything to curate it. And if a team isn’t used to making an effort to curate its history, they may assume that doing so is expensive because `git rebase -i` is scary and not something they use on auto-pilot for a few seconds at a time. In other news, our developers also create several small PRs every day but each is for an incomplete change that doesn’t stand alone so we’re never quite sure which features are finished in any given build, everyone keeps complaining about being interrupted to do code reviews all the time when the code reviews have no value anyway because they always just say LGTM :+1:, and we have targets that no more than 15% of commits should break production when CI/CD deploys them and that we recover fully within an hour each time that happens. If only there were something we could do to improve all this…
    - watwut 3y ago
      I worked in a team where tech lead insisted on nice history. It was a lot of effort all the time and very little to no benefit. He was lead and could influence salaries, his opinion mattered. So, in real life, people rarely pushed back. That is not the same as us sharing the same opinions tho. I became more verbal about history not being useful online.
    - 3y ago
Terr_ 3y ago
> even if so far you had wip wip doh wip as the commit messages Aside, `git commit --fixup HEAD` is often better than `git commit -m "oops, one more thing"`, since it means you can easily `git rebase -i --autosquash`.

WorldMaker 3y ago

If merge points are your "known good" points anyway you can just use the powers of the git dag and `git bisect --first-parent` in your main branch to just bisect the merge points. There's no need for rebase/squash and you still get useful git bisect results.

bonzini 3y ago
All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.
- WorldMaker 3y ago
  Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in development work-in-progress).
  - bonzini 3y ago
    I don't do full CI for every commit but I do run the relevant unit tests (or all of them depending on the change and the project) and ensure that they pass.
  - dkubb 3y ago
    I think the point the GP message is making is that, prior to review/merge you extract atomic commits from your WIP that tell a clear, concise story of how the change was made. The reviewer has less built up context so by chunking it like this they can step through each commit one at a time. IMHO the expectation is that each commit would 100% pass CI, so if you decided to extract some commits and merge that early you can. This is especially useful when a 6 commit PR is reviewed, and the first 3 commits are fine but there is more feedback on the last three. The reviewer can split the first 3 good ones out, get them merged and whittle down the PR to the remaining three. The subsequent follow up will be less. IME team velocity goes up with this too, and it encourages small and easy to review commits like a Remove to be extracted and merged early. Since PRs are always as large or larger than commits, I would much rather have a specific commit flagged than have to wade through the whole PR diff. If the PR is not familiar to me, I want to increase my effectiveness narrowing down the cause, so I can fix it faster.

watwut 3y ago

Frankly, people lately spend more time managing commit history then using it. Like, commit history is useful once in a year little bit, maybe, but we spend absurd amount of time trying to make it look nice.

gwright 3y ago
Not my experience, nor my team's experience over almost 10 years of using this approach.
- Chris_Newton 3y ago
  I’m firmly in your camp on this one, but I’ve noticed that advocating a tidy history gets a lot of push-back online. I think there is an element of self-fulfilling prophecy here. If a team habitually leaves a messy history behind, that history is rarely going to be useful, so naturally the team has low expectations and sees little value in doing anything to curate it. And if a team isn’t used to making an effort to curate its history, they may assume that doing so is expensive because `git rebase -i` is scary and not something they use on auto-pilot for a few seconds at a time. In other news, our developers also create several small PRs every day but each is for an incomplete change that doesn’t stand alone so we’re never quite sure which features are finished in any given build, everyone keeps complaining about being interrupted to do code reviews all the time when the code reviews have no value anyway because they always just say LGTM :+1:, and we have targets that no more than 15% of commits should break production when CI/CD deploys them and that we recover fully within an hour each time that happens. If only there were something we could do to improve all this…
  - watwut 3y ago
    I worked in a team where tech lead insisted on nice history. It was a lot of effort all the time and very little to no benefit. He was lead and could influence salaries, his opinion mattered. So, in real life, people rarely pushed back. That is not the same as us sharing the same opinions tho. I became more verbal about history not being useful online.
  - 3y ago

Terr_ 3y ago

> even if so far you had wip wip doh wip as the commit messages Aside, `git commit --fixup HEAD` is often better than `git commit -m "oops, one more thing"`, since it means you can easily `git rebase -i --autosquash`.