How to review pull requests revisions?

Question

Let's say Joe is working on a big task. He submits an initial revision of his work in a big PR with 20 commits. Now other developers spend a long time reviewing that PR and ask for a few changes/fixes. Joe now amends his commits with the requested changes and (force) pushes the new revision of the PR.

Now reviewers need to re-review the whole thing from scratch, even if the new changes were pretty minor. I haven't found a way to deal with this problem.

In the past, I've used Phabricator, which keeps track of every revision of a PR, so that you can diff changes between different revisions and solves this problem beautifully (https://secure.phabricator.com/D13641?vs=32966&id=32967#toc). For example, it lets you compare changes from revision 1 with revision 2 ). That way, you can only focus on reviewing the few new changes, rather than the 20 commits.

Is there a way we can accomplish something similar with Github? I imagine that there's a different workflow that we're not aware of. If not, do people have any alternatives to github? (apart from Phabricator?).

The only alternative I can think of is not amending commits, creating new commits instead and not force pushing. The problem with this is that the commit history becomes super messy, and there will be commits with bugs that are fixed in later commits.

Attaching below a capture of revision diffs in Phabricator, for those unaware of what I'm talking about.

thanks in advance for any advice!

Answer 1

I do not have an answer for GitHub.

What people sometimes do with the patch series (via email) workflow[1] is that they provide the output of git-range-diff(1) for each round of review after the initial one. More concretely that will mean that you store the state of your branch as a lightweight tag (store the initial “version 1”, store the “version 2” when you send that out, and so on). Then you can use that command:

git range-diff main feature-v1 feature-v2

They also describe how each series version is different from the preceding one.

For example, in this patch series, the author describes the current version as:

Changes since v6[3]:

 * Glen pointed out that ejecting a commit in v6 orphaned a
   corresponding forward-reference in a commit message, fix that.

And the range-diff says that this is the only change:

Range-diff against v6:
 1:  43fdb0cf50c =  1:  9f297a35e14 config tests: cover blind spots in git_die_config() tests
 2:  4b0799090c9 =  2:  45d483066ef config tests: add "NULL" tests for *_get_value_multi()
 3:  62fe2f04e71 !  3:  a977b7b188f config API: add and use a "git_config_get()" family of functions
    @@ Commit message
         "int" instead of "void". Let's leave that for now, and focus on
         the *_get_*() functions.

    -    In a subsequent commit we'll fix the other *_get_*() functions to so
    -    that they'll ferry our underlying "ret" along, rather than normalizing
    -    it to a "return 1". But as an intermediate step to that we'll need to
    -    fix git_configset_get_value_multi() to return "int", and that change
    -    itself is smaller because of this change to migrate some callers away
    -    from the *_value_multi() API.
    -
         1. 3c8687a73ee (add `config_set` API for caching config-like files, 2014-07-28)
         2. https://lore.kernel.org/git/xmqqczadkq9f.fsf@gitster.g/
         3. 1e8697b5c4e (submodule--helper: check repo{_submodule,}_init()
 4:  e36303f4d3d =  4:  3a5a323cd91 versioncmp.c: refactor config reading next commit

How to use this on forges like GitHub

Like I said I have no GitHub-specific answer. So this will all be informal/manual procedure.

• Make the initial PR
• Make a tag <branch-name>-v1
  • Push these tags also so that the reviewers can use them
  • Optional: Make an annotated tag and store the series-version description in the tag message
• Change your branch (rewrite) based on feedback
• Make a tag <branch-name>-v2 and force-push the changes
• Update the PR description to mention the current version and the differences from the previous one
  • The “differences” part will be the same as the one in the annotated tag if you incorporated that optional step
• Repeat until done

Notes

1. Where you use git format-patch and git send-email to send out proposed changes for review. First you send out an initial series, a “version 1”. Then when that has been discussed you send out a “version 2” based on that feedback, and so on until it gets included by whoever you want to include your changes.