4 ms·

Stacked reviews is a very natural pattern if you're using a system like Gerrit, where the unit of review is not a whole branch, but individual commits. Since th

Stacked reviews is a very natural pattern if you're using a system like Gerrit, where the unit of review is not a whole branch, but individual commits. Since they are commits, the reviews can have any kind of relationship that commits can have. I think it's a great system, including for some other reasons (it encourages amending and rebasing commits during the review process, which results in a very clean git history). After using Gerrit professionally for several years I wonder how I ever lived with GitHub-style pull requests.

baq 4y ago
github pull request review process, UI and the whole experience is very much the worst thing about github for me. there's literally one feature that I like (C-g, suggest a one-click committable change in a comment) and everything else is better in gerrit.
dimal 4y ago
How is reviewing individual commits a good thing? Maybe I’m misunderstanding something, but my branches have tons of micro commits that ultimately lead to a larger change. Reviewing each individual commit in that type of branch seems like it would be very tedious, and not useful. The reviewer would be forced to follow my entire process, with all the dead ends and refactors along the way. Seems like it would take a lot longer. Or is there another workflow that I’m not seeing?
- dodobirdlord 4y ago
  Squash your commits once you've finished prior to sending them for review? Code review of individual commits generally stems from a workflow that expects that each individual commit will contain a change that is complete in and of itself, along with the corresponding tests for that change, so that the project builds and all tests pass at every commit. This makes exploring the change history easier and reverting changes cleaner, and simplifies bisecting the commit history.
- Groxx 4y ago
  With this sort of system you generally squash your commits together to form one-per-reviewable-thing. Or at least you rewrite things a bit to make it approachable. E.g. I split things apart so one commit is an obviously-correct refactor and the later ones are pure changes. I understand some of the "no, merges are superior because they retain all history" proponents (not claiming you are one, just aiming to head off that flood), but all those dead ends and refactors along the way are just those - dead ends and refactors. They ruin the ability to bisect, for instance, because they are rarely all functional. Intentional commits, like a stacked workflow supports in a tool like Gerrit, lets you have a wonderfully understandable history that works at every point.
  - crubier 4y ago
    “They ruin the ability to bisect” is something I find myself constantly fighting. Just use “—-first-parent” in git log!!

baq 4y ago

github pull request review process, UI and the whole experience is very much the worst thing about github for me. there's literally one feature that I like (C-g, suggest a one-click committable change in a comment) and everything else is better in gerrit.

dimal 4y ago

How is reviewing individual commits a good thing? Maybe I’m misunderstanding something, but my branches have tons of micro commits that ultimately lead to a larger change. Reviewing each individual commit in that type of branch seems like it would be very tedious, and not useful. The reviewer would be forced to follow my entire process, with all the dead ends and refactors along the way. Seems like it would take a lot longer. Or is there another workflow that I’m not seeing?

dodobirdlord 4y ago
Squash your commits once you've finished prior to sending them for review? Code review of individual commits generally stems from a workflow that expects that each individual commit will contain a change that is complete in and of itself, along with the corresponding tests for that change, so that the project builds and all tests pass at every commit. This makes exploring the change history easier and reverting changes cleaner, and simplifies bisecting the commit history.
Groxx 4y ago
With this sort of system you generally squash your commits together to form one-per-reviewable-thing. Or at least you rewrite things a bit to make it approachable. E.g. I split things apart so one commit is an obviously-correct refactor and the later ones are pure changes. I understand some of the "no, merges are superior because they retain all history" proponents (not claiming you are one, just aiming to head off that flood), but all those dead ends and refactors along the way are just those - dead ends and refactors. They ruin the ability to bisect, for instance, because they are rarely all functional. Intentional commits, like a stacked workflow supports in a tool like Gerrit, lets you have a wonderfully understandable history that works at every point.
- crubier 4y ago
  “They ruin the ability to bisect” is something I find myself constantly fighting. Just use “—-first-parent” in git log!!