Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Stacked Diffs versus Pull Requests

jg.gg

Post reply on HN
Print view View original

31–40 of 77 posts

Re: Stacked Diffs versus Pull Requests

#31

Post by lima »

IshKebab wrote: post #21

This is silly. I use Phabricator and I don't commit my changes directly to `master`. I always make a branch, exactly the same as you would for pull requests. And I have multiple commits on that branch. Why? Firstly it means I can still pull master and rebase my changes to update them fairly easily. Secondly it means I can work on more than one thing at a time (who only ever has one task in progress?), thirdly it mean…

I also use Phabricator like you do, but the author makes a compelling argument to use a stack of master commits rather than individual branches.

> Firstly it means I can still pull master and rebase my changes to update them fairly easily.

Like the author says, you can do this, but it's easier when your commits are stacked on master (you only do one rebase vs. rebasing each of your branches).

> Secondly it means I can work on more than one thing at a time (who only ever has one task in progress?)

Read the article, the author uses interactive rebases to edit commits.

> finally having a change split into several commits can make code review easier

How? The reviewer doesn't see those commits, unless you submit them as separate revisions, in which case they wouldn't be on the same branch.

> As far as I can tell the only significant difference between these two approaches is that with `arc` the history in the repository ends up nice and linear, without loads of branches.

Either approach would have the same result, it's all about how things are organized in your local repository.

I'm not fully convinced (the tooling isn't exactly built for this workflow and you lose "arc feature"), but I like the idea.

Re: Stacked Diffs versus Pull Requests

#32

Post by Sean1708 »

TomK32 wrote: post #29
pbh101 wrote: post #7

Earlier quoted context omitted.

This is also suboptimal. I’d rather each commit be an atomic unit: no larger and no smaller than it needs to be. Edit: in other words, concurring with TFA.

yes, but often on a feature branch you have WIP, change your code a few commits later and so on. I use rebase -i a lot and it make better code.

`rebase -i` is fine (`commit --fixup` and `rebase --autosquash -i` is better), squashing all commits down into one is not (at least doing it all the time is not).

Re: Stacked Diffs versus Pull Requests

#33

Post by lima »

donatj wrote: post #24
petters wrote: post #6

> How do you git bisect a codebase where every 6th commit doesn’t build because it was jammed into the middle of a Pull Request? This I think is the main problem with pull requests. A better default would be squashing the commits into a single one after review.

I have always been of the mind that every commit should be a working state. My coworkers who often commit with the message “WIP” disagree. I doubt I’ll ever sell them on it.

Yeah...WIP commits are fine in your local feature branch. I wouldn't ever want to see a WIP commit in my origin/master.

Re: Stacked Diffs versus Pull Requests

#34

Post by mullsork »

lima wrote: post #28
peterjmag wrote: post #19

I think GitHub's great, and I'm mostly okay with having to rely on it heavily at my day job, but I dislike how code reviews + PRs are necessarily tied to branches. What are the chances of GitHub introducing stacked diffs (or a similar abstraction) as an option? It'd be great to try out this workflow on a non-trivial project, but it seems like a hard sell organizationally when it involves migrating to and supporting a…

You can do stacked diffs on GitHub by always using squash merges and rebasing on top of master, and it's a great way to get started. It's just a lot nicer with the extra tooling on top that Phabricator and Gerrit provide (dependencies, command line tools, and so on).

It's true that you can do that indeed. Annoyingly, you have to manually rebase if you want to use the "merge & squash" feature. I don't understand why there's no "merge into one commit, rebase with master, merge onto master" feature. It's tedious to manually keep rebasing branches that were approved for review but have gone behind.

Re: Stacked Diffs versus Pull Requests

#36

Post by jorams »

The idea of "stacked diffs" looks like a good one, but this article doesn't do a great job of presenting them. It's way too long for the actual claims it makes. It combines two things: Working on a single branch and reviewing commits instead of branches.

Reviewing commits instead of branches makes a lot of sense, and would definitely result in a cleaner history than pull requests seem to do in practice.

The article also makes the claim "They’ll get reviewed in some order and committed in some order". If the changes in the second commit depend on the first commit, that's just as false as with pull requests that depend on each other.

The article names one concrete downside of the use of many branches: If one branch depends on the changes in another branch that's not in master yet, you end up with an annoying bunch of rebases or ugly merges. Committing everything to a single branch turns that into a single rebase.

---

So that's the content. Then there are the case studies, which are odd. Charlie the free spirited hacker magically turns from someone who...

> starts the day by branching master and spamming the branch with five commits that are only vaguely related.

into someone who

> blasts out five commits and five Diffs back to back. Each one addresses something specific.

All thanks to the magic of Stacked Diffs.

Re: Stacked Diffs versus Pull Requests

#38

Post by eequah9L »

Joky wrote: post #14
petters wrote: post #6

> How do you git bisect a codebase where every 6th commit doesn’t build because it was jammed into the middle of a Pull Request? This I think is the main problem with pull requests. A better default would be squashing the commits into a single one after review.

Bisect on first-parent? Seems like a script exists (haven't tried): https://gist.github.com/ayust/2040290

Wouldn't this mean you just find the merge that caused the regression? The merge itself might bring in an awful lot of changes (of a single topic).

Re: Stacked Diffs versus Pull Requests

#40

Post by woadwarrior01 »

saketmehta wrote: post #37

In the PR model, we can easily hook up a build system to automatically trigger builds on remote branches. How would that fit into this?

And that can be (and is) done with the stacked diffs model as well. CI is orthogonal to whether you use the PR model of the stacked diffs model.
Post reply on HN