Live data from Hacker News

In Praise of Stacked PRs

benjamincongdon.me

171–180 of 230 posts

Re: In Praise of Stacked PRs

#171

Earlier quoted context omitted.

Try out the linked tool git-branchless ( https://github.com/arxanas/git-branchless , I'm the author). It should help you restore Phabricator-like workflows. In particular, check out the `git sync`, `git restack`, and `git move` commands for handling rebasing and conflict resolution.

Unfortunately I cannot install the rust compiler tool chain or other third party binaries on my work laptop :(

You don’t have the ability to install tools on your work computer? That would be a deal breaker for me.

Re: In Praise of Stacked PRs

#172
post #109

There is active work related to teaching "git rebase" to natively support stacked branches in the Git core currently being worked on by Derrick Stolee [1]. If you "stack" your changes across multiple inter-dependent branches it looks like "git rebase" is going to learn how to update related branches using a new "update-ref" command (alongside "squash", "fixup", "exec", etc) that gets activated automatically through a…

Is that likely to help with permanent forks or will that still continue to be an issue?

Re: In Praise of Stacked PRs

#173
post #76

Earlier quoted context omitted.

Isn't this exactly why you should use stacked PRs? That's what it looks like when I break a big piece of work into smaller pieces. What's the alternative anyway? If you don't want me writing 2500 lines of code in one area, would you rather I write 10 250 line PRs in 10 different parts of the codebase instead? Is that supposed to be easier to review? Or is the rule just "don't write a lot of code in any short period o…

> don't write a lot of code in any short period of time Depending on the team this may be a completely reasonable policy.

(if you are on this team, run away as fast as you can)

Re: In Praise of Stacked PRs

#174
post #128

We have some people in our company that use tools like Graphite to implement this pattern on top of Github and I don't know if I'm a big fan. Maybe just a matter of developer discipline, but in my experience people tend to create large stacks of 3+ PRs that then take a while to resolve. Yeah sure, without these tools the code would also exist somewhere, but at least you don't have your pull request list full with PRs…

https://git-ps.sh/

This git patch stack tool is really nice. You don’t really use branches at all with it. You just build up a stack of atomic commits. At any time, you can request review of a patch, and it will cherry-pick the commit onto a new branch off of master and create a PR for it.

Re: In Praise of Stacked PRs

#175

Earlier quoted context omitted.

> Probably some arcane git magic to (interactively) rebase branch There is not really a command for that yet, short of adding a bunch of `exec` steps to your interactive rebase manually. See https://news.ycombinator.com/item?id=32217204 for an upcoming command. You might enjoy using https://github.com/gitext-rs/git-stack , which specifically tries to let you manage stacked branches locally while not exposing tons of…

Also a big fan of https://gitlab.com/wavexx/git-assembler (and previously topgit). Works with the basic idea that you can rebuild branches by combining, merging or rebasing automatically on top of others. I frequently use this to build a local branch which is an amalgamation of the main branch + required patches so that I can work (and later submit) on a clean branch without being blocked.

https://git-ps.sh/

This tool is really nice and related to these types of workflows.

Re: In Praise of Stacked PRs

#176
post #156

Stacked reviews is a very natural pattern if you're using a system like Gerrit, where the unit of review is not a whole branch, but individual commits. Since they are commits, the reviews can have any kind of relationship that commits can have. I think it's a great system, including for some other reasons (it encourages amending and rebasing commits during the review process, which results in a very clean git history…

How is reviewing individual commits a good thing? Maybe I’m misunderstanding something, but my branches have tons of micro commits that ultimately lead to a larger change. Reviewing each individual commit in that type of branch seems like it would be very tedious, and not useful. The reviewer would be forced to follow my entire process, with all the dead ends and refactors along the way. Seems like it would take a lo…

With this sort of system you generally squash your commits together to form one-per-reviewable-thing. Or at least you rewrite things a bit to make it approachable. E.g. I split things apart so one commit is an obviously-correct refactor and the later ones are pure changes.

I understand some of the "no, merges are superior because they retain all history" proponents (not claiming you are one, just aiming to head off that flood), but all those dead ends and refactors along the way are just those - dead ends and refactors. They ruin the ability to bisect, for instance, because they are rarely all functional. Intentional commits, like a stacked workflow supports in a tool like Gerrit, lets you have a wonderfully understandable history that works at every point.

Re: In Praise of Stacked PRs

#177

Earlier quoted context omitted.

It’s only necessary to push ten branches, one per commit, if you intend ten separate, let’s say, “review events” though. For that example we would have fewer than that nor would we want that many. I do stack 10+ commits fairly regularly but that may be only 1-2 reviews. (I’m not familiar with Phabricator, only Gerrit and GH/GL, as well as some more manual Workflows.)

Right, in this case, I would prefer 10 separate review events. Other than for code review purposes themselves, I would want CI to run on each of these commits individually, rather than only on the result at the end.

You can click on individual commits in a pull request on GH (and in every single git tool I know) to review each commit individually. A pull request is an abstraction over a set of commits. Sometimes it's 1:1, many times it's not. Some large companies force the issue because even though it is more rigamarole for everyone involved, they can afford it. So then people invent silly scripts and coin entire workflows just so they can "push 10 PRs all at once" because they want the efficiency back that a 1:1 commit to PR policy robbed from them. It's all quite silly. FYI many CI systems support running each commit individually, including GH and GitLab.

Re: In Praise of Stacked PRs

#178

Earlier quoted context omitted.

> don't write a lot of code in any short period of time Depending on the team this may be a completely reasonable policy.

(if you are on this team, run away as fast as you can)

No, there’s no reason to run away from a team of overenthusiastic junior developers. Teach them instead and you’ll get a team of enthusiastic skilled developers, you just need to make sure they don’t make too much of a mess in the meantime.

Re: In Praise of Stacked PRs

#179
post #165

Earlier quoted context omitted.

I can't be certain what dcow is referring to but there are a few things that point to confusion, or maybe an overambitious attempt at simplification. For example, Stacking PRs keeps the author unblocked. Authors don’t need to wait on a particular change to be merged before starting to build something on top of those changes. This will fundamentally be up to the author's git skill whether or not they are presenting th…

> This will fundamentally be up to the author's git skill whether or not they are presenting the PRs to reviewers / mergers as stacked. If they're skilled git users there's little to no cost presenting them one at a time and keeping the not-yet-PRd branches fresh. On multiple teams I've worked on, this has been explicitly discouraged because reviewers often want to see the changeset in as much context as is available…

What's better context than presenting all the related atomic commits at one time in a single pull request so you know exactly which related set of commits are being added and for what reason? Slap a merge commit on and you can revert the whole thing! But your commits are a mess, and I want to see clean ones. That is what interactive rebase is for and what attentive project contributors would do in order to clean up before or during the merge process. Why is it fair to ask someone to change syntax formatting to but not cleanup their commits? Because the latter is a "big ask" for someone who doesn't know their tool.

Re: In Praise of Stacked PRs

#180
post #8

I've usually kept a rule that you should avoid stacking, and if you must only one level deep. The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough. Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.

The trick with long lived branches is to merge upstream changes frequently so you don't fall behind and keep your branch in a mergeable state. Git was designed for code bases much larger than what most people deal with where long lived branches are just a reality.

Like the Linux kernel for example. You don't get to push half finished work with projects like that. Actually, you don't get to push at all. There is only pull. Some branches exist for many months or even years and there could be entire teams collaborating on them. The one absolute certainty you have on such branches is that there is an absolutely insane amount of upstream changes all the time. The only way to stay on top of that is to merge those changes often.

You can think of the Linux kernel git as a decentralized network of stacked branches that have a few central people pulling changes from branches they have reviewed into their own branches with the one that Linus Torvalds maintains as the ultimate branch on which releases get tagged. The vast majority of changes land to his branch via multiple layers of other people, each with their own branches and each adding their own reviews. Effectively all changes Linus Torvalds integrates are stacked. And he doesn't integrate them unless they are stacked properly with nice clean histories.

You could say, git was explicitly designed to do stacked branches at scale. So, it's kind of ironic that people are re-discovering this as a thing. It always was intended to be used like this. It's been used like that since the very beginning.

Post reply on HN