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

GitHub Stacked PRs

github.github.com

Post reply on HN
Print view View original

261–270 of 548 posts

Re: GitHub Stacked PRs

#261

Post by mhh__ »

I think the core conceptual difference between a stacked diff and PRs as we use them in open source is the following:

A PR is basically a cyberspatial concept saying "I, as a dog on the internet, am asking you to accept my patches" like a mailing list - this encourages trying to see the truth in the whole. A complete feature. More code in one go because you haven't pre-agreed the work.

Stacks are for the opposite social model. You have already agreed what you'll all be working on but you want to add a reviewer in a harmonious way. This gives you the option to make many small changes, and merge from the bottom

Re: GitHub Stacked PRs

#262

Post by ivantop »

sam_bristow wrote: post #256
calebio wrote: post #106

Earlier quoted context omitted.

I miss the Phabricator review UI so much.

What does Facebook use internally these days. I'm amazed that the state of review tools is still at or behind what we had a decade ago for the most part.

It’s still phabricator

Re: GitHub Stacked PRs

#263

Post by ragall »

outworlder wrote: post #128
forrestthewoods wrote: post #104

Earlier quoted context omitted.

Mercurial has a strictly superior API. The issue is solely that OG Mercurial was written in Python. Git is super mid. It’s a shame that Git and GitHub are so dominant that VCS tooling has stagnated. It could be so so so much better!

> The issue is solely that OG Mercurial was written in Python. Are we back to "programming language X is slow" assertions? I thought those had died long ago. Better algorithms win over 'better' programming languages every single time. Git is really simple and efficient. You could reimplement it in Python and I doubt it would see any significant slowness. Heck, git was originally implemented as a handful of low level…

> I thought those had died long ago.

No, it's always been true. It's just that at some point people got bored and tired of pointing it out.

Re: GitHub Stacked PRs

#264

Post by sam_bristow »

ivantop wrote: post #262
sam_bristow wrote: post #256

Earlier quoted context omitted.

What does Facebook use internally these days. I'm amazed that the state of review tools is still at or behind what we had a decade ago for the most part.

It’s still phabricator

Any idea if their internal version has improved dramatically since they stopped maintaining the public version?

Re: GitHub Stacked PRs

#265

Post by jmalicki »

outworlder wrote: post #128
forrestthewoods wrote: post #104

Earlier quoted context omitted.

Mercurial has a strictly superior API. The issue is solely that OG Mercurial was written in Python. Git is super mid. It’s a shame that Git and GitHub are so dominant that VCS tooling has stagnated. It could be so so so much better!

> The issue is solely that OG Mercurial was written in Python. Are we back to "programming language X is slow" assertions? I thought those had died long ago. Better algorithms win over 'better' programming languages every single time. Git is really simple and efficient. You could reimplement it in Python and I doubt it would see any significant slowness. Heck, git was originally implemented as a handful of low level…

Every time I've rewritten something from Python into Java, Scala, or Rust it has gotten around ~30x faster. Plus, now I can multithread too for even more speedups.

Python is absurdly slow - every method call is a string dict lookup (slots are way underused), everything is all dicts all the time, the bytecode doesn't specialize at all to observed types, it is a uniquely horrible slow language.

I love it, but python is almost uniquely a slow language.

Algorithms matter, but if you have good algorithms, or you're already linear time and just have a ton of data, rewriting something from a single-threaded Python program to a multithreaded rust program I've seen 500x speedups, where the algorithms were not improved at all.

It's the difference between a program running overnight vs. in 30 seconds. And if there are problems, the iteration speed from that is huge.

Re: GitHub Stacked PRs

#266

Post by landr0id »

steveklabnik wrote: post #118
smallmancontrov wrote: post #70

Earlier quoted context omitted.

I'm so glad git won the dvcs war. There was a solid decade where mercurial kept promoting itself as "faster than git*†‡" and every time I tried it wound up being dog slow (always) or broken (some of the time). Git is fugly but it's fast, reliable, and fugly, and I can work with that.

What is kind of funny here is that you're right locally. At the same time, the larger tech companies (Meta and Google, specifically) ended up building off of hg and not git because (at the time, especially) git cannot scale up to their use cases. So while the git CLI was super fast, and the hg CLI was slow, "performance" means more than just CLI speed. I was never a fan of hg either, but now I can use jj, and get som…

>At the same time, the larger tech companies (Meta and Google, specifically) ended up building off of hg and not git because (at the time, especially) git cannot scale up to their use cases.

Fun story: I don't really know what Microsoft's server-side infra looked like when they migrated the OS repo to git (which, contrary to the name, contains more than just stuff related to the Windows OS), but after a few years they started to hit some object scaling limitations where the easiest solution was to just freeze the "os" repo and roll everyone over to "os2".

Re: GitHub Stacked PRs

#267

Post by skydhash »

MrJohz wrote: post #180
mikeocool wrote: post #138

Earlier quoted context omitted.

Constantly rewriting git history with squashes, rebases, manual changes, and force pushes has always seemed like leaving a loaded gun pointed at your foot to me. Especially since you get all of the same advantages with plain old stream on consciousness commits and merges using: git merge --no-ff git log --first-parent git bisect --first-parent

I find rebases are only a footgun because the standard git cli is so bad at representing them - things like --force being easier to write than --force-with-lease, there being no way to easily absorb quick fixes into existing commits, interdiffs not really being possible without guesswork, rebases halting the entire workflow if they don't succeed, etc. I've switched over pretty much entirely to Jujutsu (or JJ), which…

The magic of the git cli is that it gives you control. Meaning whatever you want to do can be done. But it only gives you the raw tools. You'll need to craft your own workflow on top of that. Everyone's workflow is different.

> So while I'm working on my branch, I can iteratively make a[...]which means git blame tends to be much more accurate and helpful

Everything here I can do easily with Magit with a few keystroke. And magit sits directly on top of git, just with interactivity. Which means if I wanted to I could write a few scripts with fzf (to helps with selection) and they would be quite short.

> And while I'm working on a branch, I still have access to the full history of each commit...

Not sure why I would want the history for a specific commit. But there's the reflog in git which is the ultimate undo tool. My transient workspace is only a few branches (a single one in most cases). And that's the few commits I worry about. Rebase and Revert has always been all I needed to alter them.

Re: GitHub Stacked PRs

#268

Post by ninkendo »

steveklabnik wrote: post #250
ninkendo wrote: post #248

Earlier quoted context omitted.

> This is the model that the kernel uses Nah. The kernel uses a mailing list, and a “review” means a mailing list thread. With some nice CLI tools to integrate with git when you want to actually apply the patch (or start a review thread.) In that world, “[PATCH 2/5]” (or whatever) in the subject title, and a different CC list for each patch, is a nice way to be able to ensure different subsets of the patch series hav…

Sorry, I am talking about stacked diffs in general, not this specific implementation on GitHub. That "Patch 2/5" is five stacked diffs, on top of each other. Forges that are stacked-diff native do that same kernel flow, just on the web instead of over email. You can also see this corroborated over here: https://news.ycombinator.com/item?id=47758251 All of the advantages, like "it’s a simple threaded discussion betwee…

> Sorry, I am talking about stacked diffs in general, not this specific implementation on GitHub

My point is that the LKML and what GitHub do is so different that the definition of “stacked diffs in general” can only describe a tiny aspect of each, if you want to call both of their approaches by the same name. From where I sit, the only common element between them is “they offer a way to keep discussion separated.”

If that’s all people are actually complaining about, there are a thousand better ways to “keep discussion separated” that don’t require me to pretend that it’s ok that only a subset of my branch is ok to merge.

In git, a branch is the thing you either merge or don’t. You merge multiple commits at once, or you don’t. It’s a great model. Breaking up the branch into smaller pieces, and giving people the impression it’s ok to merge the first commit but not the rest, just to unfuck the discussion UX, is putting the cart before the horse. I make a branch strictly because I want it to either all merge or none of it merge. It’s the only sensible approach in my book. If a discussion system is so bad that this is unworkable, it means the discussion system is bad, it doesn’t mean the conceptual model of a merge is bad.

Re: GitHub Stacked PRs

#269

Post by skydhash »

akersten wrote: post #126
xixixao wrote: post #92

Earlier quoted context omitted.

Conflicts spawn most likely because PR A was squashed, and once you squash Git doesn't know that PR B's ancestors commits are the same thing as the squashed commit on main. No idea if this feature fixes this. Edit: Hopefully `gh stack sync` does the rebasing correctly (rebase --onto with the PR A's last commit as base)

> Conflicts spawn most likely because PR A was squashed, and once you squash Git doesn't know that PR B's ancestors commits are the same thing as the squashed commit on main. Yeah, and I kind of see how git gets confused because the squashed commits essentially disappear. But I don't know why the rebase can't be smart when it sees that file content between the eventual destination commit (the squash) is the same as t…

Because at first your have this

  main
Then you'll have

  main, squashed A
      \
       \-> PR A -> PR B
The tip of B is the list of changes of both A and B, while the tip of main is now the squashed version of the changes of A. Unless a branch tracks the end of A in the PR B, It looks like more you want to apply A and B on top of A again.

A quick analogy to math

  main is X
  A is 3
  B is 5
Before you have X + 3 + 5 which was equivalent to X + 8, but then when you squash A on on X, it looks like (X + 3) + (3 + 5) from `main`'s point of view, while from B, it should be X + (3 + 5). So you need to rebase B to remove its 3 so that it can be (X + 3) + 5.

Branches only store the commits at the top. The rest is found using the parent metadata in each commits (a linked list. Squashing A does not remove its commits. It creates a new one, and the tip of `main` as its parent and set the new commit as the tip of `main`. But the list of commits in B still refer to the old tip of `main` as their ancestor and still includes the old commits of A. Which is why you can't merge the PR because it would have applies the commits of A twice.

Re: GitHub Stacked PRs

#270

Post by steveklabnik »

ninkendo wrote: post #268
steveklabnik wrote: post #250

Earlier quoted context omitted.

Sorry, I am talking about stacked diffs in general, not this specific implementation on GitHub. That "Patch 2/5" is five stacked diffs, on top of each other. Forges that are stacked-diff native do that same kernel flow, just on the web instead of over email. You can also see this corroborated over here: https://news.ycombinator.com/item?id=47758251 All of the advantages, like "it’s a simple threaded discussion betwee…

> Sorry, I am talking about stacked diffs in general, not this specific implementation on GitHub My point is that the LKML and what GitHub do is so different that the definition of “stacked diffs in general” can only describe a tiny aspect of each, if you want to call both of their approaches by the same name. From where I sit, the only common element between them is “they offer a way to keep discussion separated.” I…

> My point is that the LKML and what GitHub do is so different that the definition of “stacked diffs in general” can only describe a tiny aspect of each

That's fine, what I mean is, when we started this convo, I thought you were asking about the general concept of stacked diffs, not the specifics of what GitHub is releasing here. That's my mistake for misunderstanding, sorry about that.

This is also (assumedly, anyway) why they're calling this "stacked PRs" and not "stacked diffs," because what they're doing is slightly different than Gerrit, Phabricator, Critique, etc.

Post reply on HN