|
The words you're saying are correct in isolation, but in context of the thing you're replying to, 1) At issue here is not (just) the portion of the history in the changes under review, but the choice of where that history starts. That's what the commenters above mean by "diff based" and "branch based". If I start a Git branch on HEAD@{yesterday}, and HEAD@{today} renames a function that I'm using, then there is a semantic difference between approving solely my changes and approving my branch with information about its branch point. And in a workflow where only my changes are under review without further history, it's entirely possible for me to submit a set of changes that simply don't sensibly apply to a specific target branch, because the tooling did not reflect that my source branch is different. Note that all of this true even if "my changes" takes the form of a series of patches, such as a [PATCH m/n] series posted to a mailing list using an LKML-style workflow! 2) All that said, I do not know of any tool that makes it particularly easy to manage both of these pieces of history in a review. All the Git-based review tools I've seen (GitHub, GitLab, Gitea, etc.) make it fairly easy to see where the remainder of history is, view the final state of a modified file with history (e.g., it's easy to see the blame view of a modified file), etc. But they don't default to showing you the commit-by-commit history; they show you the diff between the beginning and end for review. Meanwhile, the [PATCH m/n] workflow (e.g., git format-patch, including Sourcehut's tooling on top) is in fact much closer to the Phabricator one in spirit; while it does permit you to see the commit-by-commit history, it has no idea what the history before the first patch is, and it's difficult to even tell if a patch got merged! So in a practical sense, there isn't really a tool that lets you work with full history. You have to make some form of compromise. In practice, I think the most common compromise is to constrain yourself to making one commit per code review; if the history is complex enough to really want commit-by-commit review, you're probably best off making multiple reviews anyway. (You might object - what if the code doesn't build or work right between your first and second commits? But that's already a bad place to be, because ordinary git bisect will have false build failures if it lands between those two points. There is git bisect --first-parent, but first, your project had better be doing --no-ff merges to make this useful, and second, the effect of it is to ignore the detailed history, so if you're going to ask people to do that, why burden them with the detailed history in the first place?) The other compromise here, weirdly enough, is Phabricator itself. Because Phabricator reviews diffs and not branches, you can submit two independent code reviews for two dependent commits, and Phabricator won't care. If you try to do this with any of the pull-request-style tools, the second code review will include both changes because they know history; Phabricator doesn't, and it will show just the second change if all you sent it is "arc diff HEAD^". And then you can "arc land" those on your own. (But you have to be careful about getting the order right, because, again, Phabricator doesn't know how to do that for you.) |