Improve workflow when force-pushing during code reviews

[sebastianst]

The Github Flow article encourages pushing new commits while iterating over a pull requests during code reviews. However, this sometimes leads to a messy commit history and some teams prefer to continuously force-push clean commit histories rather than pushing little review commits.

This workflow doesn't work well with Github yet, unfortunately. In 2018, Github added a hyperlink to the word force-pushed in the conversation timeline to show the diff after a force-push, which is already a great improvement.

The following points are still missing in the Web UI to support smooth force-push review iterations:

• Add review comments to individual line(s) in the comparison view. Clicking on force-pushed shows the diff, but no line-comments can be made.
• Show all PR versions/heads in the comparison view, so that individual PR versions can be compared and reviewed.
• Show old line-comments on new force-pushed versions, if possible. This requires some sort of smart algorithm to match a line-comment to the new version.

Happy to add more points. Note that Gitlab and other platforms have better support for this kind of workflow for some time now.

Related issue: isaacs/github#999

If you want to see these features being implemented, please vote by clicking on the ⬆️ above the counter. Giving a 👍 does not count as a vote, see also #3474

[sebastianst]

Edit: Moved into "General Feedback" category, where it is well-placed. So the below discussion is obsolete.

Not sure if this was the right place to post this feature request 👉 Reposted to github.community.

[evi-liu]

Thank you for reposting to GitHub community! This space is currently reserved for GitHub Discussions feedback only.

[sebastianst]

Are there plans to open up github/feedback to other categories like the Pull Requests UI? I think the Discussions interface is nicer than github.community for discussions like these.

[aspiers]

See also isaacs/github#1985

[sebastianst]

I moved this post into the "General Feedback" category, where it should be well placed. This makes my original comment obsolete.

[victorlin]

I would also like to see less clutter on the PR page when iterating on in-progress changes. Especially for draft PRs – I'm reluctant to open drafts with incomplete changes to avoid clutter on the PR page. This defeats the purpose of having a "draft" state.

In Azure DevOps, there is a similar amount of verbosity in commits/force-pushes, but it's not so bad because the PR page displays updates with most recent at the top. I prefer that over GitHub PRs where you have to scroll all the way down just to see the most recent changes/comments.

[hazz0003]

Azure DevOps some how has the same information density with about half the clutter.

[Empact]

Documented my prior experience with this in #8808 - in my experience it's a significant source of friction between people who want to keep the review history and others who want to optimize for the source history.

[victorlin]

Github added a hyperlink to the word force-pushed in the conversation timeline to show the diff after a force-push, which is already a great improvement.

I don't know if this was a recent change, but the designers somewhat addressed the verbosity issue here by compressing back-to-back force-pushes with a message <username> force-pushed the <branch> branch n times, most recently from <commit> to <commit>.

However, the force-pushed link and Compare button only show the most recent force-push diff. There's no way to view the prior force-pushes, so this feels like an incomplete feature now.

[alpipego]

This should absolutely be expandable so each individual force push diff can be viewed. Moreover, often it would be more useful to show the diff of all the recent force pushes. I think the way the UI is done currently, it's impossible to get the individual hashes.

[danilaml]

Yes. It's actually a UX downgrade, since if one force-push was a rebase, and another was a fixup, it would combine their diffs into unreviewable mess, instead of comparing rebase with fixup.

[alpipego]

One more thing that would be really helpful: The diff should not show rebased commits from the target branch of the PR.

If a feature branch gets rebased on the target branch and there are also code changes in the feature branch, the diff for the force pushes shows the changes of all commits from the target branch as well.

Since they have already been reviewed and merged, it's often very hard if not impossible for reviewers to identify the actual code changes of the push.

[Hnasar]

I sure hope this gets implemented one day! I think this is an area that GitLab does better than GitHub.

git on the command line offers range-diff (docs) which properly shows the diff between two sequences of commits and is a way that sensible changes can be seen, even in the face of force pushes and rebases.

This would be a killer code review feature for GitHub if it offered it in the web UI.