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

41–50 of 77 posts

Re: Stacked Diffs versus Pull Requests

#41

Post by Ajedi32 »

So basically the author proposes replacing Pull Requests with "Cherry-Pick Requests", where each commit is a single reviewable unit (replacing separate branches with the PR model), the local master branch represents a list of unreviewed commits, and the remote master branch represents reviewed, "merged" commits.

It's an interesting idea, but even after reading this article I'm still not convinced it offers any serious advantages over a more traditional workflow. It also has the downside of requiring extensive use of more advanced features like interactive rebasing. (Which I personally have no issue with, but seems like a potential nightmare for developers who aren't as experienced with git.)

Re: Stacked Diffs versus Pull Requests

#43

Post by Ajedi32 »

woadwarrior01 wrote: post #40
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.

But what commit do you run the CI against? If you run it against a commit that's stacked on top of 3-4 other unreviewed changes, then the CI will include errors from all those other commits, not just yours.

Re: Stacked Diffs versus Pull Requests

#44

Post by agapon »

It seems that topgit can be used to achieve this workflow. The beauty of it is that it is built on top of git, so it has the familiar mechanics internally while providing a new way of doing patch series / stacked diffs.

Re: Stacked Diffs versus Pull Requests

#46

Post by dom96 »

yumaikas wrote: post #39

How does this workflow work when there are 3 commits that need to be applied in order (A, B, and C), when C gets approved from a code-review standpoint before A and B?

You are prevented from merging commits at the top of the stack until the commits below are accepted.

The advantage is that each commit can be reviewed separately. You cannot achieve this with pulled requests as far as I know. I often see people creating a pull request and writing "this depends on PR #1233", the second PR is unreviewable because it contains changes from. The first PR. So the first PR has to merged first... It's a mess. But maybe I am just missing a big part of the PR workflow that solves this problem.

Re: Stacked Diffs versus Pull Requests

#47

Post by dom96 »

Ajedi32 wrote: post #43
woadwarrior01 wrote: post #40

Earlier quoted context omitted.

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.

But what commit do you run the CI against? If you run it against a commit that's stacked on top of 3-4 other unreviewed changes, then the CI will include errors from all those other commits, not just yours.

You don't stack against random commits. Only commits that have passed tests before and so are on master, or your own commits.

Re: Stacked Diffs versus Pull Requests

#48

Post by krupan »

jorams wrote: post #36

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 a…

You are missing the subtle and hard to explain point that he tried to illustrate with the Tale of the Tree Icon. It's that our tools and their defaults shape the way we work. I watched a team switch from branches to stacked diffs (mercurial queues, way back in the day) and it was almost exactly how this article describes.

Re: Stacked Diffs versus Pull Requests

#49

Post by zbentley »

eequah9L wrote: post #38
Joky wrote: post #14

Earlier quoted context omitted.

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).

I think that’s desirable. First and foremost, you want to know whose work (in this case, a merged branch) broke yours. Afterwards you might want to drill into what specific piece (in this case commit) of that work was responsible, which can be achieved by bisecting the branch’s non-auto-merge/non-rebase history; this usually gets you close enough to “where the bug was actually written” in my experience, without requiring a switch to a specific workflow of what a commit is supposed to represent/how committed code should behave.
Post reply on HN