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

Stacked PRs are now live on GitHub

github.blog

Post reply on HN
Print view View original

301–308 of 308 posts

Re: Stacked PRs are now live on GitHub

#301

Post by beaker52 »

IshKebab wrote: post #281
beaker52 wrote: post #243

Earlier quoted context omitted.

You can read each individual commit and review the PR as a whole. This is the way many people design and review PRs. You don’t need to merge those changes progressively. If you do, you go through exactly the same process of creating a separate branch and PR. The only difference is that GH has now added some UI and automation for rebasing and merging the PRs. In the past we would have explained the chaining in the PR…

Look at some level you want a "thing that is reviewed and has CI run on it" right? Unless you work alone you need that. Let us call that unit of work, a flob. When you have written and submitted a flob for review, you often want to continue your work on top of that, and then you may end up with a second dependent flob that is finished before the first flob is merged . You want both to be reviewed. You want CI to run…

> Look at some level you want a "thing that is reviewed and has CI run on it" right?

Yeah, it’s called a branch, or PR. It’s a set of changes you want to sign off.

It seems like you want CI run on every commit, which seems rather unnecessary. And if you don’t, well, that’s always been the case.

> Not in a way that worked properly.

GitHub operates git. No git changes have happened. It’s just commits and branches, in git. So anything that worked before, works exactly the same now, but with buttons taking out some of the small amount of effort you had to put in.

Re: Stacked PRs are now live on GitHub

#303

Post by IshKebab »

beaker52 wrote: post #301
IshKebab wrote: post #281

Earlier quoted context omitted.

Look at some level you want a "thing that is reviewed and has CI run on it" right? Unless you work alone you need that. Let us call that unit of work, a flob. When you have written and submitted a flob for review, you often want to continue your work on top of that, and then you may end up with a second dependent flob that is finished before the first flob is merged . You want both to be reviewed. You want CI to run…

> Look at some level you want a "thing that is reviewed and has CI run on it" right? Yeah, it’s called a branch, or PR. It’s a set of changes you want to sign off. It seems like you want CI run on every commit, which seems rather unnecessary. And if you don’t, well, that’s always been the case. > Not in a way that worked properly. GitHub operates git. No git changes have happened. It’s just commits and branches, in g…

[dead]

Re: Stacked PRs are now live on GitHub

#304

Post by sunshowers »

Degorath wrote: post #300
sunshowers wrote: post #294

Earlier quoted context omitted.

Indeed, and I happened to have been part of the creation story for one of them :) https://lobste.rs/s/ghdv6w/i_see_future_jj#c_nx6flj (not otherwise affiliated, though)

That's really cool! I tried getting on board with ERSC, but the timezone difference proved too much :(. At least it gave me the kick to try and build my own company, though!

That's wonderful! Good luck.

Re: Stacked PRs are now live on GitHub

#305

Post by nijave »

happimess wrote: post #127
nijave wrote: post #109

Earlier quoted context omitted.

>Can't you run your CI locally? If you mean automated tests and linters, sure. Conceptually continuous integration is generally integrating 2+ different lineages of code together which is more common with multiple developers although I suppose it's becoming more relevant with agents creating a bunch of worktrees with different things. In practice, CI has taken the same path as "DevOps Engineer" ie most people just me…

> Conceptually continuous integration is generally integrating 2+ different lineages of code together > In practice, CI [often means] "automated test server" Could you say more? I use "CI" to refer to the automated processes that run tests and (maybe) deploy code as it is merged to some blessed branch. It's what continually integrates the new code into the existing code. What am I missing?

If there's 1 developer working on 1 branch at a time: main is at commit A, you branch add B and C, you merge now main is at A, B, C. Your "integration" was just moving the code forward and if you ran tests on A, B, C commits and it passed, the tests would still pass on main.

Now say there's 2 developers that branch off A. Dev 1 removed dead code coolUtil and with it deleted module coolLib and commits B1. Tests pass, code works. Dev 2 adds new coolUtil2 in commit B2. Tests pass, code works. Now they both merge their changes back to branch A so it's A, B1, B2 or A, B2, B1 or A, B3 (merge of B1/2). Anyway, you try to run the code and it fails. Everything merged clean but Dev 1 removed the lib Dev 2 depends on. They didn't create any file level conflicts because they edited different things so the merging went fine however now the code is broken.

The real integration part comes from reconciling different (potentially conflicting) changes and making sure the end result still works. The server is doing the integration automatically and verifying it automatically

You could skip the server and merge everything on your machine and run the tests on your machine and achieve the same result. Still CI, no server, no automation

Hopefully that makes sense, if not I think the Wikipedia page "Practices" section does a decent job separating the pieces https://en.wikipedia.org/wiki/Continuous_integration

Re: Stacked PRs are now live on GitHub

#306

Post by othmanosx »

m11a wrote: post #249
Okkef wrote: post #17

What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit? I think the bigger problem is that big AI PR's need a different way of reviewing. For example, the order in which the diff's are shown can make a big difference in how easy the commits are to read (e.g., function definition change first, then all call sites, then the tests). Or maybe we should go to a system w…

GitHub's PR workflow doesn't nicely support being able to review individual commits, realise which comments are associated with which commits, etc. Or shipping individual commits to main, while working on some others (unless you allow cherry-picking and direct push to main). Or amending a certain commit with respect to feedback and seeing a diff from the previous patch of that commit to the next. If GitHub's unit of…

have you tried pyor.review? it is actually just github but with better UI, and you get a commits rail that makes it easy to switch between commits on the fly

Re: Stacked PRs are now live on GitHub

#307

Post by othmanosx »

I've been using it for a while and I've actually shipped stacked PRs with github. we usually try to chunk the work so that we don't get in a situation were we have to open PRs with more 1k+ LOC, and we try to plan chunks of max 500 LOC, but I do find myself getting in this situation when the planned work turns out to be more than I anticipated like refactoring or edge cases or review feedback... etc. but that's where stacked PRs come in, I found that they could be useful in this situation to split an unexpectedly large PR into a few smaller ones that are chunkable, individually shippable, and easy to review. but that's where the good stuff ends IMO, stacked PRs just make a bad situation slightly better, and I think we shouldn't get into this situation in the first place. I know that can be impossible sometimes, but I already built my own solution for fix this problem.

even though it's good to have, Github's UI doesn't help it, Github already sucks in terms of the review experience in a pull request and the implementation of the stacked PRs leaves a lot to be desired.

thankfully, I was working on my own solution for this problem and I already integrated a better stacked PRs implementation compared to github, and I'm pretty happy with it.

Pyor (pyor.review), I built it myself, sits on top of github, syncs everything with it, and gives me the benefit of having a better UI and code review experience.

Post reply on HN