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

201–210 of 548 posts

Re: GitHub Stacked PRs

#201

Post by jstimpfle »

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…

[flagged]

Re: GitHub Stacked PRs

#203

Post by jstimpfle »

forrestthewoods wrote: post #143
outworlder wrote: post #128

Earlier quoted context omitted.

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

They died because everyone knows that Python is infact very very slow. And that’s just totally fine for a vast number of glue operations. It’s amusing you call Git fast. It’s notoriously problematic for large repos such that virtually every BigTech company has made a custom rewrite at some point or another!

Now that is interesting too, because git is very fast for all I have ever done. It may not scale to Google monorepo size, it would ve the wrong tool for that. But if you are talking Linux kernel source scale, it asolutely, is fast enough even for that.

For everything I've ever done, git was practically instant (except network IO of course). It's one of the fastest and most reliable tools I know. If it isn't fast for you, chances are you are on a slow Windows filesysrem additionally impeded by a Virus scanner.

Re: GitHub Stacked PRs

#204

Post by ninkendo »

steveklabnik wrote: post #131
ninkendo wrote: post #107

> a chain of small, focused pull requests that build on each other — each one independently reviewable. I have never understood what this even means. Either changes are orthogonal (and can be merged independently), or they’re not. If they are, they can each be their own PR. If they’re not, why do you want to review them independently? If you reject change A and approve change B, nothing can merge, because B needs A t…

The canonical example here is a feature for a website that requires both backend and frontend work. The frontend depends on the backend, but the backend does not depend on the frontend. This means that the first commit is "independent" in the sense that it can land without the second, but the second is not, hence, a stack. The root of the stack can always be landed independently of what is on top of it, while the res…

> If you do this in a PR, both get assigned to review the whole thing. Each person sees the code that they don't care about, because they're grouped together.

There are two separate issues you’re bringing up:

- Both groups being “assigned” the PR: fixable with code owners files. It’s more elegant than assigning diffs to people: groups of people have ownership over segments of the codebase and are responsible for approving changes to it. Solves the problem way better IMO.

- Both groups “seeing” all the changes: I already said GitHub lets you view single commits during PR review. That is already a solved problem.

And I didn’t even bring up the fact that you can just open a second PR for the frontend change that has the backend commit as the parent. Yes, the second PR is a superset of the first, but we’ve already established that (1) the second change isn’t orthogonal to the first one and can’t be merged independently anyway, and (2) reviewers can select only the commits that are in the frontend range. Generally you just mark the second PR as draft until the first one merges (or do what Gitlab does and mark it as “depends on” the first, which prevents it from merging until the first one is done.) The first PR being merged will instantly make the second PR’s diff collapse to just the unique changes once you rebase/merge in the latest main, too.

All of this is to explain how we can already do pretty much all of this. But in reality, it’s silly to have people review change B if change A hasn’t landed yet. A reviewer from A may completely throw the whole thing out and tell you to start over, or everything could otherwise go back to the drawing board. Making reviewers look at change B before this is done, is a potential for a huge waste of time. But then you may think reviewers from change B may opt to make the whole plan go back to the drawing board too, so what makes A so special? And the answer is it’s both a bad approach: just make the whole thing in one PR, and discuss it holistically. Code owners files are for assigning ownership, and breaking things into separate commits is to help people look at a subset of the changes. (Or just, like, have them click on the folder in the source tree they care about. This is not a problem that needs a whole new code review paradigm.)

Re: GitHub Stacked PRs

#205

Post by bmitc »

smallmancontrov wrote: post #70
adamwk wrote: post #25

As someone who used phabricator and mercurial, using GitHub and git again feels like going back to the stone ages. Hopefully this and jujutsu can recreate stacked-diff flow of phabricator. It’s not just nice for monorepos. It makes both reviewing and working on long-running feature projects so much nicer. It encourages smaller PRs or diffs so that reviews are quick and easy to do in between builds (whereas long pull…

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.

Git is not remotely fast for large projects.

Re: GitHub Stacked PRs

#206

Post by bmitc »

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…

You barely have to try to have Python be noticeably slow. It's the only language I have ever used where I was even aware that a programming language could be slow.

Re: GitHub Stacked PRs

#207

Post by SamuelAdams »

ezekg wrote: post #9
bsimpson wrote: post #5

Finally! I never understood the PR=branch model GitHub defaulted to. Stacked commits (ala Phabricator/Gerrit) always jived more with how my brain reasons about changes. Glad to see this option. I guess I'll have to install their CLI thing now.

My only complaint off the bat is the reliance on the GH CLI, which I don't use either. But maybe by the time it's GA they'll have added UI support.

CLI is great because now I can tell my AI agent to do it. “Fix all dependabot security issues (copy logs) and run tests to validate functionality. Create each dependency as its own stack (or commit) so that contributors may review each library update easily.”

Wait 10 minutes and you’re done.

Re: GitHub Stacked PRs

#208

Post by bmitc »

ezekg wrote: post #9
bsimpson wrote: post #5

Finally! I never understood the PR=branch model GitHub defaulted to. Stacked commits (ala Phabricator/Gerrit) always jived more with how my brain reasons about changes. Glad to see this option. I guess I'll have to install their CLI thing now.

My only complaint off the bat is the reliance on the GH CLI, which I don't use either. But maybe by the time it's GA they'll have added UI support.

Why don't you use the CLI?

Re: GitHub Stacked PRs

#209

Post by steveklabnik »

ninkendo wrote: post #204
steveklabnik wrote: post #131

Earlier quoted context omitted.

The canonical example here is a feature for a website that requires both backend and frontend work. The frontend depends on the backend, but the backend does not depend on the frontend. This means that the first commit is "independent" in the sense that it can land without the second, but the second is not, hence, a stack. The root of the stack can always be landed independently of what is on top of it, while the res…

> If you do this in a PR, both get assigned to review the whole thing. Each person sees the code that they don't care about, because they're grouped together. There are two separate issues you’re bringing up: - Both groups being “assigned” the PR: fixable with code owners files. It’s more elegant than assigning diffs to people: groups of people have ownership over segments of the codebase and are responsible for appr…

> fixable with code owners files.

Code owners automatically assigns reviewers. You still end up in the state where many groups are assigned to the same PR, rather than having independent reviews.

> I already said GitHub lets you view single commits during PR review.

Yes, you can look at them, but your review is still in the context of the full PR.

> And I didn’t even bring up the fact that you can just open a second PR for the frontend change that has the backend commit as the parent.

The feature being discussed here is making this a first-class feature of the platform, much nicer to use. The second PR is "stacked" on top of the first.

Post reply on HN