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.
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?
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.
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?
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.
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...
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.
Comments
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.
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?
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.
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.
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?
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.
Cool, thanks.
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...
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.
Not once they hit master, no. You push bug fix commits.