5 ms·

Because it is visual noise. It's really hard to track, especially on the CLI. It's also a lot easier to reason about the codebase through time if it's basically

by kortex 4y ago

Because it is visual noise. It's really hard to track, especially on the CLI. It's also a lot easier to reason about the codebase through time if it's basically a linear track. We started off with merge commits and moved to squash rebase (team history to team clean), and I gotta say, I absolutely love team clean in the overwhelming majority of cases. The only time team history is handy is as a sort of temporary space on feature branches where you are sorting out complex merges, debugging, or trying different patterns. But all of that gets squashed away onto main eventually.

ilammy 4y ago
> It's really hard to track, especially on the CLI. git log --first-parent --max-parents=1 Now you don't see merge commits. Forbid merge commits with conflict resolution, and you're done.
- Gigachad 4y ago
  How do you forbid the ones with resolution? Genuine question because it seems like merge commits are a black hole where anything could happen and no one would notice.
  - seba_dos1 4y ago
    Just require the merged branch to be fast-forwardable (but don't actually fast-forward it). If it's not, it will be obviously visible in `git log --graph`.
- seba_dos1 4y ago
  You usually don't want to use both arguments at the same time as that will hide the whole merges (both merge commits and merged commits). You can however use one of those arguments to choose what you want to filter out, which is very handy. FWIW, `--max-parents=1` has an alias `--no-merges`.
raggi 4y ago
git log --topo-order
- deleted 4y ago
  [deleted]
seba_dos1 4y ago
I can perfectly understand the appeal of keeping the history clean by requiring ff-only merge commits, but I fail to see any proper case where squash rebase is actually useful in any way. It only loses potentially useful information and doesn't give anything in return. The only usefulness I can image getting out of it is throwing away garbage commit messages and fixup commits made by people who didn't bother to get them right, which sounds like something to fix in the first place. When you want to maintain linear history, FF-only merge commits allow you to have cake (keep properly atomic changes retained) and eat it (filter out the detail in git log when it's unwanted). Squashing (eating cake) or rebasing with no merge commit (having cake) only makes things worse from there.
- Gigachad 4y ago
  Every PR I have worked on starts with some good commits but then they become useless when review happens and a bunch of commits that are essentially “do the suggested thing”. Better to just squash it all in to one commit for the whole PR. Ideally the PR is small enough to be logical as one commit.
  - seba_dos1 4y ago
    > Better to just squash it all in to one commit for the whole PR. Absolutely not. It's better to edit your PR to contain suggested changes in a clear, atomic history before presenting it for another round of review. Why would I want to present commits that "do the suggested thing" to the reviewer in the first place? That doesn't make any sense and it shouldn't pass the review.
    - agsnu 4y ago
      Having a commit in isolation that "does the suggested thing" reduces the cognitive load on the reviewer(s) because you can see the conversation history and the specific changes in response to that comment without having to re-parse the entire branch changeset from first principles. Basically the trade off is, re-write history before review or after. If you re-write before review, you lose some context about the evolution of the changeset - also the GitHub UI can get confusing (because some conversation comments will apply to commits that no longer exist on the PR branch). Keeping the pre-review iteration visible in the context of the PR can make it easier for someone else to come along and join the discussion later. If your feature branches/PRs are short-lived, then the size of a squashed merge is still quite small, and the pre-review iteration is irrelevant. If for some reason the pre-review iteration is interesting, it's still there in the context of the PR, which is back-referenced from the commit message.

by kortex 4y ago

ilammy 4y ago

> It's really hard to track, especially on the CLI. git log --first-parent --max-parents=1 Now you don't see merge commits. Forbid merge commits with conflict resolution, and you're done.

Gigachad 4y ago
How do you forbid the ones with resolution? Genuine question because it seems like merge commits are a black hole where anything could happen and no one would notice.
- seba_dos1 4y ago
  Just require the merged branch to be fast-forwardable (but don't actually fast-forward it). If it's not, it will be obviously visible in `git log --graph`.
seba_dos1 4y ago
You usually don't want to use both arguments at the same time as that will hide the whole merges (both merge commits and merged commits). You can however use one of those arguments to choose what you want to filter out, which is very handy. FWIW, `--max-parents=1` has an alias `--no-merges`.

raggi 4y ago

git log --topo-order

deleted 4y ago
[deleted]

seba_dos1 4y ago

I can perfectly understand the appeal of keeping the history clean by requiring ff-only merge commits, but I fail to see any proper case where squash rebase is actually useful in any way. It only loses potentially useful information and doesn't give anything in return. The only usefulness I can image getting out of it is throwing away garbage commit messages and fixup commits made by people who didn't bother to get them right, which sounds like something to fix in the first place. When you want to maintain linear history, FF-only merge commits allow you to have cake (keep properly atomic changes retained) and eat it (filter out the detail in git log when it's unwanted). Squashing (eating cake) or rebasing with no merge commit (having cake) only makes things worse from there.

Gigachad 4y ago
Every PR I have worked on starts with some good commits but then they become useless when review happens and a bunch of commits that are essentially “do the suggested thing”. Better to just squash it all in to one commit for the whole PR. Ideally the PR is small enough to be logical as one commit.
- seba_dos1 4y ago
  > Better to just squash it all in to one commit for the whole PR. Absolutely not. It's better to edit your PR to contain suggested changes in a clear, atomic history before presenting it for another round of review. Why would I want to present commits that "do the suggested thing" to the reviewer in the first place? That doesn't make any sense and it shouldn't pass the review.
  - agsnu 4y ago
    Having a commit in isolation that "does the suggested thing" reduces the cognitive load on the reviewer(s) because you can see the conversation history and the specific changes in response to that comment without having to re-parse the entire branch changeset from first principles. Basically the trade off is, re-write history before review or after. If you re-write before review, you lose some context about the evolution of the changeset - also the GitHub UI can get confusing (because some conversation comments will apply to commits that no longer exist on the PR branch). Keeping the pre-review iteration visible in the context of the PR can make it easier for someone else to come along and join the discussion later. If your feature branches/PRs are short-lived, then the size of a squashed merge is still quite small, and the pre-review iteration is irrelevant. If for some reason the pre-review iteration is interesting, it's still there in the context of the PR, which is back-referenced from the commit message.