5 ms·

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 lar

by 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!!

by dimal 4y ago

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!!