Reinventing the pull request
lubeno.dev
lubeno.dev
Why would I want to build on changes that haven't been reviewed and accepted? That's a good way to waste my time having to redo something because the foundation it was based on was flawed and got rejected later.
There's a reason git and most accepted development workflows are linear.
Also, I don’t have time to wait around for a review to work on other parts of the same codebase.
So I branch of a previous PR instead of development when the changes are related, and continue development, and rebase after the previous PR has been merged.
Usually this leads to less rework and fewer merge conflicts.
Very occasionally it means having to rework subsequent work because the approach was actually wrong, but I'll take that over the hassle of continuing to work from a base I know is already outdated.
There are so many times where I want to create 3-4 Merge requests that all build on each other along the same branch instead of creating one giant MR but the UI for reviewing them doesn't really work that way.
it sounds like you want multiple checkpoints for review parts to happen on, but still only 1 merge in the end
Stacked diffs just let you review one commit at a time, rather than requiring a full review of the entire thing, while still letting you see that these changes are related, so you can go look at the full context if you'd like.
This lets you do things like "hey, backend person, please review this backend commit, frontend person, please review this frontend commit" instead of "both of you review this full PR". This can save the frontend person time if, for example, the frontend code passes review, but the backend reviewer hasn't had time to review the backend changes yet. They won't be pinged by the other review.
Not to waste time not being able to do anything that depends on those changes?
> git and most accepted development workflows are linear.
Linear like branches easily branching off other branches?
The way github does PRs is great for open source projects where you have submissions from a very diverse set of developers. You need to evaluate the whole PR as whole and accept it or reject it.
But it is not ideal where you have a small team working on single product. The PR review becomes a gate and it has been my experience as a developer that you spend a LOT of time trying to get your PR reviewed.
My preferred approach is to do smaller and more frequent commits that are merged directly to the main branch. Developers learn to break a problem into very small changes which is a skill by itself. It is the responsibility of a lead or Sr engineer to check the commits as they come so they can provide feedback as soon as possible. This was the spirit of the original continuous integration ideas back in the day.
Basically, I agree with you that large PRs are a problem, I just don't think it means you need to throw pre-land code review out the window.
But yeah, if the team lead is aware of what everyone is working on, and prioritizes fast CLs review, huge amounts of friction and slowdown are removed from the process
Other systems, like Gerrit, handle this much better!
I've fixed this issue in several teams I've joined. Maybe some people won't understand this, as I didn't until I observed it multiple times over.
There are teams out there, perhaps the majority, that simply leave the decision as to when a PR will be reviewed nebulous, and at the discretion of team members. There is no formal obligation to do them in a timely manner, and there are no consequences if they are not done.
The solve to this is obvious and easy:
- Automatically assign specific people to PRs. No general team assignment. The submitter can add specific people in addition to PR's if they need domain experts, but the normal case is random assignment.
- Require PR's to be done within 24 working hours. If you cannot do this for whatever reason, you must communicate this to the team.
- There are consequences if you violate this policy.
The last one is the hard part for cowardly teams and cowardly managers. You do have to stay on top of it initially, and even when people have accepted this and gotten used to it, you can't forget about it because people will drift.
This isn't to say I'm against the direct integration model you propose, I can also see the appeal, it's just that problems with PR flows are mostly about cowardly management, not anything much to do with the actual process.
We also paired this with giving the on call engineer near total freedom with what to do during their shifts, similar to 20% time (which was about the same percentage; 5 team members, weekly on call rotations.) They chose which tickets to pick up from the backlog, which also helped keep up with maintenance and taking care of bugs and issues that otherwise wouldn't get prioritized.
Though if it can just make stacked PRs widespread, I'd already be very happy.
I also appreciate the ordering. In my projects, we put an extra focus on tests by having having a commit that adds new tests to reproduce the bad behavior so when you diff the tests with the fix commit, you get a visual of how things changed.
I also find that the order can be PR specific. I wonder about allowing the contributor or reviewrs reorder on per-PR basis.
There are also times we have a lot of test or doc changes. I wonder about grouping items to jump between or collapsing to more easily navigate around than opening and browsing a file picker.
I haven't signed up, but I'm not convinced the stacked PR's is a good route. This is mostly because I'm thinking about it in terms of how I work now, so maybe after a trial run I would see it differently. My experience with launching software is people don't want to change their workflows.
So the way I imagine how this would get used by real accounts is they will still review individual PR's and merge them in like normal and avoid doing stacked PR's because a lot of us mostly care about the final result and less about the how you got there.
That is until we need to understand the how lol So being able to visualize this is probably a very useful tool for retroactive inspection over being part of the PR review workflow.
I hope the author doesn't feel like I'm dunking on them, I really love the idea of re-thinking PR.
Maybe this tool would help, but nothing in this pitch convinces me.
To the OP: JS is for enhancements. If you are using it in such a way that it becomes a potential blocker for all of the content then you are not going to reach the audience that you otherwise would. If you are on somebody else's platform/framework then by all means pass this up the chain.
https://pagespeed.web.dev/analysis/https-lubeno-dev-blog-rei...
[0] https://ersc.io/
That said, we haven’t talked a ton publicly about what exactly we’re building yet, because we’re very focused on building it. But if you or anyone else is dealing with pain around source control management, I’d love to hear about it: steve@ersc.io
(This is a genuine question, I think local first review is very interesting, but at some point, you've gotta send that feedback to the person you're reviewing.)
I have found that this no longer needs to be an issue with agentic coding tools.
Once I am happy with the end state of a branch, I tell Claude to rebuild the change from scratch as a set of atomic incremental commits. It adds about 2 minutes to the dev process, but creates a pr that is infinitely easier to review.
The overall thrust of the article is great, though. The tooling around prs needs a ton of attention.
The file priority thing is a great idea too. That would be even more useful for search. The number of times I'm searching for something on Github and it just shows me a gazillion tests.... Yeah you can look up their advanced search syntax and exclude them probably, but it's always a hassle.
The workflow just makes it a pain, since you have to manually rebase in both the branch and the UI after the original PR merges.
Next you create another branch `feature_1`, based on `feature_0`. You push `feature_1` to yourname/fancyproject. There is now no way to make a PR for `feature_1` that includes only the changes from feature_0 to feature_1. You can't do it because when you create the PR (in fancyproject/fancyproject) you can't select `feature_0` as a base branch because that branch doesn't exist in fancyproject/fancyproject.
Are you're saying that you'd want the commits for `feature_0` through `feature_1` to be in the PR (to the upstream fancyproject repo), and you'd want it to exclude the commits in the range from the fork commit through to `feature_0`?
While that's a very edge case scenario, I think (theoretically) you'd be able to do it if you first cherry-pick the commits you want onto a new branch based upon the same original fork commit that your repo forked from.
ie ensure the PR branch you create doesn't include any commits you want excluded
That should pretty much guarantee the PR goes smoothly. In theory. :)
That's what stacked PRs are
You can review just the addition, knowing what it is building on top of.
(this was a follow-up to the initial announcement months ago, also made via X)
The T-101 PR can't really be reviewed yet, since you are looking at changes from both T-100 and T-101 (because T-101 was based on T-100).
Ideally, after T-100 is reviewed and merged, the T-101 PR would automatically update to show only the T-101 changes. But it doesn't. You have to manually rebase or merge main and push changes to the branch to get it to update. It would be great if GitHub handled this automatically.
This would be especially helpful for mobile, where reading a traditional diff is basically impossible.
https://lubeno.dev/assets/images/blog/lubeno-repository.png
https://lubeno.dev/assets/images/blog/lubeno-pr.png
https://lubeno.dev/assets/images/blog/code-review-before-aft...
I believe that the ideas in the blog post are novel enough and should spark curiosity and interesting discussions. Also I submitted this last week, someone must have hand-picked and given it another chance because it's a good fit for HN.
Thanks for disclosing the financial conflict of interest, but this doesn't change the self-promotion factor.
Right now there are effectively three ways to do a PR:
- a bunch of small commits, some of them related to the feature, some fixes, some mixing both -> a PR with 'n' commits -> they don't really make sense as atomic commits, you have to review the entire PR to make the sense of it
- a squashed PR
- some uber principled reorganisation of commits that separates key implementation concerns into smaller commits (effectively stacked PRs but clean)
The last option would be desirable but it's unreasonable to expect anyone to do it by hand. So this is where <maybe> an LLM could parse my garbage intermediate commits, the final diff and generate a stack instead?
As a fellow rebase enjoyer, I will do it occasionally for smaller PRs but to me, it becomes unwieldy for large ones.
Do you have any tips or aliases that makes it more workable?
You don’t have to do it this way, you could use the oplog to undo it all if it’s not to your satisfaction, but it’s much simpler than trying to edit the commits into the right shape directly.
Are you seriously comparing LLMs with a deterministic code generator?
Definitely. Even the current leading edge AI models (ie Claude Code Opus 4.6 (1M)) gets things (badly) wrong occasionally, makes bad decisions, and does stupid stuff.
It's a lot better than even just a few months ago, but blinding accepting its output for code that matters will catch you out.
Take a look at `git rebase --interactive`.
Case in point:
> dd to cut a line and p to paste it in a new position is a very quick way to reorder)
It isn't quick, you're just swiping the whole issue under the rug - first, you need the whole separate interface, but more importantly, this new interface is very primitive, you see close to no context, only some commit names, so it's not quick to find what to move and where because the content for those decisions is in a different place. Sure, you could add some vim plugin that expands it and adds per-commit info (what, you want to view the diff for all 3 commits you selected and DDed? Tough luck, you don't see the lines anymore! And even if you did, that's not this plugin), but then it's not your `--interactive` git "core" that does convenience
I've got some wheels spare, care to have some?