Code review tools should really compare with reviewable.io, which supports proper review of every-commit in a PR, with force pushes, making sure all changes get read, and comment sign-off and disposition, making sure no comment remains unaddressed.
In contrast to Gerrit and Phabricator, it needs not "Change IDs" inserted in your commits (easier workflow just using git) and "just works" to review whole branches.
It seems to me that "1 PR = 1 commit = 1 review" and "stacked PRs" workflows are just workarounds for not properly having implemented that as Reviewable has. Am I not seeing something?
Reviewable's main drawback is being for Github only and not open source.
Help me understand why I care about reviewing the fifteen commits my junior developer did while figuring out how to make a SQL query, and not just the final line of code? Typically, all I really care about is what's actually going into production, not the journey they took to get there. So, what am I missing?
Gerrit/CodeApprove/Reviewable-style reviews are actually designed for exactly the scenario you're describing.
The thing you're missing is that it's helpful to see a diff view of, "What changed since my last review?"
If your review workflow is:
1. Junior engineer makes 15 commits to implement a feature in 300 LOC
2. Junior engineer sends you the PR for review
3. You review and send your notes to the engineer
4. Junior engineer makes 15 more commits and another 100 LOC churn, but PR is 350 LOC total diffs
At (4), the thing you probably want to see are the 100 LOC of diffs since step (3). I haven't tried this on GitHub for awhile, but last I checked, your options are to either view only diff of PR against main branch, view each of the 15 commits individually, or hand edit the URL to get the "what's changed since (3)?" view.
On Gerrit/CodeApprove/Reviewable, they all default to "what changed since I last reviewed?" and you comment on that diff rather than what's changed against the main branch, which is the default on GitHub.
Then, when your senior developer is working on a new feature that requires some changes to adapt to a dependency upgrade, some refactoring, some forwards-and-backwards compatible database migrations, you'll appreciate a stack of discrete, clean, working, individually reviewable commits.
Comments
Code review tools should really compare with reviewable.io, which supports proper review of every-commit in a PR, with force pushes, making sure all changes get read, and comment sign-off and disposition, making sure no comment remains unaddressed.
In contrast to Gerrit and Phabricator, it needs not "Change IDs" inserted in your commits (easier workflow just using git) and "just works" to review whole branches.
It seems to me that "1 PR = 1 commit = 1 review" and "stacked PRs" workflows are just workarounds for not properly having implemented that as Reviewable has. Am I not seeing something?
Reviewable's main drawback is being for Github only and not open source.
Help me understand why I care about reviewing the fifteen commits my junior developer did while figuring out how to make a SQL query, and not just the final line of code? Typically, all I really care about is what's actually going into production, not the journey they took to get there. So, what am I missing?
Gerrit/CodeApprove/Reviewable-style reviews are actually designed for exactly the scenario you're describing.
The thing you're missing is that it's helpful to see a diff view of, "What changed since my last review?"
If your review workflow is:
1. Junior engineer makes 15 commits to implement a feature in 300 LOC
2. Junior engineer sends you the PR for review
3. You review and send your notes to the engineer
4. Junior engineer makes 15 more commits and another 100 LOC churn, but PR is 350 LOC total diffs
At (4), the thing you probably want to see are the 100 LOC of diffs since step (3). I haven't tried this on GitHub for awhile, but last I checked, your options are to either view only diff of PR against main branch, view each of the 15 commits individually, or hand edit the URL to get the "what's changed since (3)?" view.
On Gerrit/CodeApprove/Reviewable, they all default to "what changed since I last reviewed?" and you comment on that diff rather than what's changed against the main branch, which is the default on GitHub.
Ah, if true, than I misunderstood and agree. That didn't sound like what the person I replied to was saying though.
It's hard to leave Gerrit after using it.
Those would be squashed into one commit.
Then, when your senior developer is working on a new feature that requires some changes to adapt to a dependency upgrade, some refactoring, some forwards-and-backwards compatible database migrations, you'll appreciate a stack of discrete, clean, working, individually reviewable commits.