4 ms·

The use case is when you look at a branch as a series of patches. Reviewing a clean set of commits is much easier than a branch full of mistakes and wrong path

by gorset 2y ago

The use case is when you look at a branch as a series of patches. Reviewing a clean set of commits is much easier than a branch full of mistakes and wrong paths taken. Useful when we optimize for reviewing and good history for future maintenance. This has been important and useful when I’ve worked on big mission critical backend system, but I also understand it might not be the most important factor for a new project’s success.

dclowd9901 2y ago
So this wouldn’t work very well in workflows that flatten merges to a trunk?
- travisjungroth 2y ago
  It would mostly be unnecessary. The separate commits won’t matter after the PR if they’re getting squashed. Debatably, if you’re making changes during a PR review, it could be helpful to make those changes in relevant commits. That way if someone goes through them during the PR, they get one clean “story”, rather than see the pre-PR commits and the conversation after.
- mst 2y ago
  I like for non-trivial stuff to have a branch with a series of logical commits, cleanly rebased atop main, then use -no-ff to force a merge commit anyway. That way you can the whole branch appears as a single commit/diff atop main in primary history but you can dig in to the original components of it if/when that's useful. The primary obstacle to doing this for me is, if I'm honest, not having automated it sufficiently that I can't forget to do that.
- chippiewill 2y ago
  Tools where they allow you to squash merge tend to be fairly incompatible with stack diff workflows generally. It's tricky to review individual commits on Github and Gitlab and they don't even allow you to leave comments on the commit messages. In areas where people do review individual commits they tend to use tools that support that workflow. Git uses email patches where each commit is sent as a separate email. Tools like Gerrit do code review directly on a commit by commit basis and the usual strategy to get it into trunk is to either "fast forward" (advance the HEAD of the trunk branch to that commit) or to cherry-pick it.
keybored 2y ago
A branch with some base is already a series of commits. I don’t get where the conceptual re-imagining is here.
- gorset 2y ago
  Patch series comes from the linux kernel workflow, which git was developed to support. https://kernelnewbies.org/PatchSeries https://kernelnewbies.org/PatchSeries In this workflow you review every commit and not just the branch diff. Each commit is crafted carefully, and a well crafter series of commits can make even very large changes a brief to review. It takes a certain skill to do this well. As the page above says > Crafting patches is one of the core activities in contributing code to the kernel, it takes practice and thought. This is in contrast to using git more as a distributed filesystem where you don't care particularly much about the history, and you typically squash commits before merging etc. It's simpler and easier to work this way, but you lose some of the nice attributes of the linux kernel workflow.
  - keybored 2y ago
    That’s a nice summary. What I don’t like about the Git documentation as I’ve read it is that they go between “patch” and “commit” in some places without stopping and explaining what the difference is. It makes sense to them. It’s obvious. But it isn’t necessarily obvious to most people. A patch is a patch proper plus a commit message encoded in a format that git am understands. That’s fine. And the core developers understand that you cannot transmit a commit snapshot via email (or you shouldn’t). But I prefer to mostly stick to “commit” in the abstract sense, whether that to-be-commit is from a pull or from an email (or: it’s in the form of an email and it could be applied as a commit). git rebase talks about “patch series” I think. Without explaining it. Why not “commit series”? Sometimes it seems like talking about your changes by the way it happened to be transmitted. It’s like talking about “attachments” instead of commits because you happened to send them via email as an attachment (instead of inline). Then you now have “stacked diffs” or “stacked commits”. Which are just a series of commits. Or a branch of commits (implicitly grounded by a base commit). For a while I was wondering what stacked diffs/stacked PRs/stacked patches and if I was missing out. When it just turned out to be, as you explain, essentially the Linux Kernel style of being able to review a commit in isolation. But in a sort of context that pull request inhabitants can understand. I prefer to mostly talk about these things as “commits”. (At several times writing those paragraphs above I wondered if I would be able to string together them in a coherent way)

by gorset 2y ago

dclowd9901 2y ago

So this wouldn’t work very well in workflows that flatten merges to a trunk?

travisjungroth 2y ago
It would mostly be unnecessary. The separate commits won’t matter after the PR if they’re getting squashed. Debatably, if you’re making changes during a PR review, it could be helpful to make those changes in relevant commits. That way if someone goes through them during the PR, they get one clean “story”, rather than see the pre-PR commits and the conversation after.
mst 2y ago
I like for non-trivial stuff to have a branch with a series of logical commits, cleanly rebased atop main, then use -no-ff to force a merge commit anyway. That way you can the whole branch appears as a single commit/diff atop main in primary history but you can dig in to the original components of it if/when that's useful. The primary obstacle to doing this for me is, if I'm honest, not having automated it sufficiently that I can't forget to do that.
chippiewill 2y ago
Tools where they allow you to squash merge tend to be fairly incompatible with stack diff workflows generally. It's tricky to review individual commits on Github and Gitlab and they don't even allow you to leave comments on the commit messages. In areas where people do review individual commits they tend to use tools that support that workflow. Git uses email patches where each commit is sent as a separate email. Tools like Gerrit do code review directly on a commit by commit basis and the usual strategy to get it into trunk is to either "fast forward" (advance the HEAD of the trunk branch to that commit) or to cherry-pick it.

keybored 2y ago

A branch with some base is already a series of commits. I don’t get where the conceptual re-imagining is here.

gorset 2y ago
Patch series comes from the linux kernel workflow, which git was developed to support. https://kernelnewbies.org/PatchSeries https://kernelnewbies.org/PatchSeries In this workflow you review every commit and not just the branch diff. Each commit is crafted carefully, and a well crafter series of commits can make even very large changes a brief to review. It takes a certain skill to do this well. As the page above says > Crafting patches is one of the core activities in contributing code to the kernel, it takes practice and thought. This is in contrast to using git more as a distributed filesystem where you don't care particularly much about the history, and you typically squash commits before merging etc. It's simpler and easier to work this way, but you lose some of the nice attributes of the linux kernel workflow.
- keybored 2y ago
  That’s a nice summary. What I don’t like about the Git documentation as I’ve read it is that they go between “patch” and “commit” in some places without stopping and explaining what the difference is. It makes sense to them. It’s obvious. But it isn’t necessarily obvious to most people. A patch is a patch proper plus a commit message encoded in a format that git am understands. That’s fine. And the core developers understand that you cannot transmit a commit snapshot via email (or you shouldn’t). But I prefer to mostly stick to “commit” in the abstract sense, whether that to-be-commit is from a pull or from an email (or: it’s in the form of an email and it could be applied as a commit). git rebase talks about “patch series” I think. Without explaining it. Why not “commit series”? Sometimes it seems like talking about your changes by the way it happened to be transmitted. It’s like talking about “attachments” instead of commits because you happened to send them via email as an attachment (instead of inline). Then you now have “stacked diffs” or “stacked commits”. Which are just a series of commits. Or a branch of commits (implicitly grounded by a base commit). For a while I was wondering what stacked diffs/stacked PRs/stacked patches and if I was missing out. When it just turned out to be, as you explain, essentially the Linux Kernel style of being able to review a commit in isolation. But in a sort of context that pull request inhabitants can understand. I prefer to mostly talk about these things as “commits”. (At several times writing those paragraphs above I wondered if I would be able to string together them in a coherent way)