15 ms·

IMHO equating commits and PRs puts undue pressure on the scope and quality of a given commit, adding potential for unnecessary stress and eliminating the benefi

IMHO equating commits and PRs puts undue pressure on the scope and quality of a given commit, adding potential for unnecessary stress and eliminating the benefits of an additional buffer / layer for aggregation of changes. A PR representing a sizable feature or refactor might naturally contain a dozen commits, each dedicated to a logical area or a requisite subset of the whole. Assuming on principle a goal of keeping main in a known-good state, such intermediate and incomplete changes (fine in an unstable feature branch) would wreak havoc. It's equivalent to asking, "Why would you have more than one story in an epic (or task in a story)?".

what 22d ago
If your PR has more than one commit, each one should be deployable in isolation. Which means you can split your giant PR into smaller ones that can be reviewed independently.
- tclancy 22d ago
  I’ve worked under both systems, but isn’t the purity you’re describing a bit of a dodge in that you wind up force pushing amended commits when you find you forgot something?
  - steveklabnik 22d ago
    Why is that a dodge? that's the expected way to work in this system, and it should be able to show you the interdiff between those amends.
    - tclancy 21d ago
      Sorry, “dodge” was a loaded word. I suppose what I mean is, what’s the difference between a PR with multiple commits and a commit with multiple commits?
      - steveklabnik 21d ago
        I’m not sure what a “commit with multiple commits” is. I direct we’re talking past each other slightly :) I think this is maybe what you’re asking about: with PRs, you often respond to feedback by adding new commits on to the PR. This is because code review is based on reviewing an entire branch. In a stacked diff based system, you respond to feedback by amending the commit. This is because reviews are tied to individual commits, not branches. The reason people prefer this is that it keeps changes small and focused, and makes sure you have a high quality history. It also lets you assign different reviewers to different parts of your stack, which is helpful for all sorts of reasons. It also means that you can land earlier commits while waiting on review for the later commits, instead of holding it all up at once. Lots of stuff like this.
        tclancy 20d ago
        Cool, thanks.
        steveklabnik 20d ago
        Any time. This stuff took me a while to wrap my head around, since I never worked at Google or Meta. I don't think I'm the best at explaining it yet either...
        tclancy 20d ago
        Ha, yeah. I worked at Box.com for a couple years and it took me months to figure out why my giant PRs full of genius were getting so much pushback. Just two different workflows depending on scale.
  - adastra22 22d ago
    Not once they hit master, no. You push bug fix commits.
  - Orphis 22d ago
    People say they care about the "story" behind the PR. But no one cares about that story if it's about forgetting to fix a test and a typo in a comment. The extra commits are just noise that make you think the original commit is a source of truth in a blame when it has been amended 3 times more in the same PR, but the link isn't apparent anymore. Force pushing is bad to a published branch, not a feature branch (not that you really have force pushes in Gerrit anyway). There are versions of Gerrit tooling where you can have a branch as dirty as you want locally, but only the final aggregated change is visible for review of that's what you prefer too.
boomlinde 22d ago
In Gerrit, I would say that the equivalent of the epic is the topic, stacks are the stories and commits are the tasks. It certainly puts pressure on the scope and quality of commits, but I think that's only undue for a short-lived project where you don't foresee a long period of maintenance. Consistently high quality commits with a clear scope is a godsend when you are investigating the history of a project. Some small pressure now is IMO better than greater pressure later when you are debugging an issue at three in the morning and scrolling through either a bunch of "fix stuff" commits or 1000+ line PR squashes.
mejutoco 22d ago
> IMHO equating commits and PRs puts undue pressure on the scope and quality of a given commit, adding potential for unnecessary stress and eliminating the benefits of an additional buffer / layer for aggregation of changes You can do as many commits as you want locally. Then go back and squash them before pushing. I think that addresses all your points, if I am not mistaken.

what 22d ago

If your PR has more than one commit, each one should be deployable in isolation. Which means you can split your giant PR into smaller ones that can be reviewed independently.

tclancy 22d ago
I’ve worked under both systems, but isn’t the purity you’re describing a bit of a dodge in that you wind up force pushing amended commits when you find you forgot something?
- steveklabnik 22d ago
  Why is that a dodge? that's the expected way to work in this system, and it should be able to show you the interdiff between those amends.
  - tclancy 21d ago
    Sorry, “dodge” was a loaded word. I suppose what I mean is, what’s the difference between a PR with multiple commits and a commit with multiple commits?
    - steveklabnik 21d ago
      I’m not sure what a “commit with multiple commits” is. I direct we’re talking past each other slightly :) I think this is maybe what you’re asking about: with PRs, you often respond to feedback by adding new commits on to the PR. This is because code review is based on reviewing an entire branch. In a stacked diff based system, you respond to feedback by amending the commit. This is because reviews are tied to individual commits, not branches. The reason people prefer this is that it keeps changes small and focused, and makes sure you have a high quality history. It also lets you assign different reviewers to different parts of your stack, which is helpful for all sorts of reasons. It also means that you can land earlier commits while waiting on review for the later commits, instead of holding it all up at once. Lots of stuff like this.
      - tclancy 20d ago
        Cool, thanks.
        steveklabnik 20d ago
        Any time. This stuff took me a while to wrap my head around, since I never worked at Google or Meta. I don't think I'm the best at explaining it yet either...
        tclancy 20d ago
        Ha, yeah. I worked at Box.com for a couple years and it took me months to figure out why my giant PRs full of genius were getting so much pushback. Just two different workflows depending on scale.
- adastra22 22d ago
  Not once they hit master, no. You push bug fix commits.
- Orphis 22d ago
  People say they care about the "story" behind the PR. But no one cares about that story if it's about forgetting to fix a test and a typo in a comment. The extra commits are just noise that make you think the original commit is a source of truth in a blame when it has been amended 3 times more in the same PR, but the link isn't apparent anymore. Force pushing is bad to a published branch, not a feature branch (not that you really have force pushes in Gerrit anyway). There are versions of Gerrit tooling where you can have a branch as dirty as you want locally, but only the final aggregated change is visible for review of that's what you prefer too.

boomlinde 22d ago

In Gerrit, I would say that the equivalent of the epic is the topic, stacks are the stories and commits are the tasks. It certainly puts pressure on the scope and quality of commits, but I think that's only undue for a short-lived project where you don't foresee a long period of maintenance. Consistently high quality commits with a clear scope is a godsend when you are investigating the history of a project. Some small pressure now is IMO better than greater pressure later when you are debugging an issue at three in the morning and scrolling through either a bunch of "fix stuff" commits or 1000+ line PR squashes.

mejutoco 22d ago

> IMHO equating commits and PRs puts undue pressure on the scope and quality of a given commit, adding potential for unnecessary stress and eliminating the benefits of an additional buffer / layer for aggregation of changes You can do as many commits as you want locally. Then go back and squash them before pushing. I think that addresses all your points, if I am not mistaken.