Compare the case study of Yang, who runs either
git checkout neighbour-fix
git checkout -b yang-work
git checkout neighbour-fix
git checkout yang-work
git rebase neighbour-fix
arc patch --diff D12345 # arc's a CLI tool for Phabricator
git rebase -i HEAD~2
<goes into phabricator and finds the new commit hash for D12345>
<pastes it into vim in place of the original commit hash>
Apologies if I've misunderstood exactly which commands Yang would run in either case.
I feel like phab had a couple workflows that were very efficient and if you ever had to do multiple git operations you were probably using it wrong. You could do everything but the actual code review in the CLI. Github is hampered because they can only use git operations and they just try to figure everything out server-side.
That said, phab certainly has it's idiosyncrasies and anything outside of the workflow that they intended was not supported.
Stacked diffs make that possible because you can review either individual commits or an entire stack at once.
The irony is that this is largely the way that Linux kernel development works -- and the Linux kernel was the first user of Git! Most projects who later adopted Git seem to have never learned this lesson that was there from the beginning, and have since been reinventing wheels.
> If you want to have your code reviewed, you first have to branch master, then commit to that branch, then push it remotely, then create a PR.
When I want to create a PR, I always create a feature/WIP branch locally and push that. Branches are cheap in Git.
I find when the unit of scrutiny is an individual commit, like when you contribute to the Linux kernel, I spend way to much time fabricating nice atomic commits. Like maybe 50%, with 30% actual development time and 20% normal cleanup.
The only thing I miss with PRs is the ability to rewrite my branch after showing it. I think it is not supported to do `rebase -i` and then `push --force` to a PR, is it? It would be really useful, especially for the cases where you have multiple commits that cancel each other out (e.g. add debugging output, remove debugging output).
Don’t know about Github.
This is one problem with the "don't rewrite history" comments people make: the advise isn't clear enough, People advise against it once "push"ed or "public", but that isn't right. Its when collaborating, like on a long lived branch. PR branches are 99% of the time single contributor.
You can also construct such a diff yourself by going to something like /compare/<old commit hash>..<new commit hash>, and those hashes can also be tags or branch names, and should even allow relative stuff like HEAD^.
(I haven't checked the URL just now, but I use both often. It's a little annoyingly hidden, but it is there.)
PS: I checked out your demo. I can't see how to diff arbitrary commits, only revisions of a PR.
As for diffing commits, you can switch a review into "review each commit" mode so that it will create one revision per commit. You can make this the default for a repo but it's not in the demo as it's not the most common way to work. The demo does really need updating, though!
If there are extensive changes from a follow-up review it gets its own commit and specific commit message.
GitHub throws up 404 when I do this. Same with simply trying to view the old commit directly using the `.../commit/<hash>` link.
Things like making a branch off of another branch being somehow so difficult that no one would ever even consider it. Or that there isn't a Github CLI tool that allows you to avoid having to go to the UI.
> "a Github CLI tool that allows you to avoid having to go to the UI."
-- better yet, git-extras (https://github.com/tj/git-extras) provides CLI utils that work across github, gitlab, and bitbucket.
In both cases you can rewrite your commit stack after-the-fact to add new ones, make fixes etc.
I usually describe it by comparing it to floating patches instead of anchored branches:
(Phabricator) diffs are more like git patches than commits/branches for review. They can be "grafted" on master or other places by applying the patch anywhere master or local branch, the diff is "floating". PRs are specific commits / commit ranges to review and adopt by merging/rebase-squashing, but are "anchored" to the place they branched off of.
This matters most when fixing 1 bug means multiple separate change that would be best staying separate commits, having to make N pull requests that depend on each other leads to rebase/forcepush cascades, which sucks. Having "patches" ready to apply means that halfway through "landing" (closing/merging) the diffs, the remaining diffs can be grafted on actual master instead of the (now closed) diff below, instead of having to rebase forever to update Github UI.
One major downside of this workflow is that it's different from what git teaches you, so every newcomer needs to learn "the Phab way", which gets people grumpy, and is only realistically feasible in a workplace, not really in a FOSS project as it increases the bar to contribution.
I agree this is a poor choice, but it's not about merge strategy. In fact Github, when you apply a pull request, rebases it linearly by default and it goes into the tree the same way a "stacked diff" would, preserving the linear history feature that is the whole point of that strategy.
 I hate that term, FWIW. The value being provided is a continuous linear history in the main archive, we should call it "branchless" or something instead of naming it after minutiae about how it's implemented.
I wish this were true, but it's really not. Not remotely. It's garbage. bradfitz's twitter thread does a good job enumerating the problems. My shortlist: unable to comment on arbitrary lines; unable to comment on specific/old commits (surprise! you force pushed and they're gone anyways);
> I hate that term, FWIW.
The traditional term is "patch set" -- you apply the patches in series.
Mailing lists are truly the only good (publicly available) review tool.
Again, you're off on a tangent talking about the Github review UI and not git branching strategies. As far as github: meh. I use it every day to manage large PRs on a large project, and it's fine. Gerrit and mailing list processes fix the problems you have, but have their own pain points. Gitlab is worse than all three.
Pick the tools that work for you, adapt to the ones that work for your codevelopers, and don't start fights. Calling someone else's tool garbage helps nothing but your upvote count.
> I use it every day to manage large PRs on a large project, and it's fine.
I feel like you're becoming borderline disingenuous here. I demonstrated how Github is not fine, it's actually quite deficient, regardless of your branching strategy. The title of TFA is literally "Stacked Diffs versus Pull Requests". Here's the second line of TFA:
"People who have never used it and only use Pull Requests with GitHub/GitLabs generally don’t understand what the fuss is all about."
I'm beginning to believe you're those people.
> Pick the tools that work for you, adapt to the ones that work for your codevelopers, and don't start fights. Calling someone else's tool garbage helps nothing but your upvote count.
I'm not looking to pick a fight here, I'm honestly not sure why any sane person would defend the Github PR workflow. Beyond "it exists" it really doesn't have much going for it.
I call it Garbage because that's how I see it. And I hope others see it that way too. And I hope that we as an industry work to build tools that are not Garbage and I hope that those folks mentioned in the article learn to use and experience other tools and realize there is A Better Way to do things. We don't all have to put up with mediocre software.
If I see a friend eating literal garbage, I would tell them "hey, you're eating literal garbage, maybe you should stop" and not just keep it to myself.
PRs are expensive. So when you think 'this is a self contained change, but I need it to continue development' you are unlikely to send a PR for it. You will just pile on more code.
Because if you make another PR based on that first PR and someone finally does code review and tells you to fix something then you suddenly need to propagate these changes to all follow up PRs which is quite some work.
And while I think I might just miss something obvious it seems many people miss it that's why we have blog posts about 'stacked PRs'. (On HN a day ago or so)
Having a far greater emphasis on even smaller units of work for a single feature (per commit) seems to unnecessarily increase cognitive load? Like now I have to do even more "planning ahead" of how my changes are going to inter-relate, and find some way to separate them out. I mean, that is ideal, but... software also has to be written.
I’ve never plan out my commits and I manage to have well-structured commits. I use a mixture of git add -p and noticing when I’m needing to do something else and being diligent. It’s a skill for sure, but I think it’s one that anyone can get good at with practice and mentorship.
The easiest way actually to start this is to have a single giant WIP commit. Then git reset HEAD^ once you are done and now start committing piecemeal. Once you do this enough times you’ll develop a sixth sense and start making the interim commits on the fly and use fixups for mistakes that you autosquash. That’s what I do 90-95% of the time and the rest of the time I might have to do some rewrites or I might get some merge conflicts with myself (if I change a preceding commit that a subsequent commit changes). Here’s the thing though. If you’re being diligent about your commits in PR land and making sure each is reviewable (which really is what you should always do regardless), then you’ve already done all the work. Representing it as a stacked diff in the review tool just makes it easier to land piecemeal and avoid situations like “well your PR had other pieces but I needed this one piece from it so I extracted it into its own PR and fixed the issues I had with it. Sorry about the merge conflict”.
Stacked diffs aren’t necessarily the end of where I want review tools to get to. What I really want is to be able to generate a hydra where I might merge in entire stacks. If diffs sit in review long enough, other things I’m working on start to depend on previous commits in other stacks (eg cool feature X). I want to be able to rebase the entire stack and have the tools work correctly. Unfortunately that’s not possible with any tool I’m aware of and stacked diffs still suffer from rebase hell (Facebook internally had a GUI tool that made this slightly more bearable by letting you drag branches around visually). However, this piece requires support from both review tools and the SCM system (so I can rebase an entire body of work without doing each piece manually and resolving BS conflicts multiple times)
<plug> https://github.com/krobelus/git-branchless/ </plug>
> resolving BS conflicts multiple times
For identical conflicts you can enable git rerere
Putting stuff in commits ASAP locally is good practice anyway, since it guarantees you will never accidentally lose your work.
with svn, it was common for me to have something with 50 files.
Currently working on 5 files "v,w,x,y,z".
I noticed a simple typo in file "c":
modify "c", commit "c"
3 pending modifications in "f,g,h".
Someone ask me to correct something else in "c".
svn up, correction and commit "c".
then "svn up", just updating everything without messing with current work on "v,w,x,y,z".
(no need to stash, or rebase, or anything as no one else touched them anyway)
And let's suppose that i just want the new version file "k" without modifying everything in my current working folder:
svn up "k"
Good old time was as easy as that... even if git has its own advantages for some other cases.