There’s SO much to be done around Code Review, I’m very surprised GitHub isn’t doing more.
They have the reach already, they have years of real review data. And yet, the PR itself is not using any of the newly available methods for more efficient review.
Agents are hammering PRs out left and right, and yet here I am still staring at the raw diff of 35 PRs and end up being called the bottleneck.
At least for me. Am I missing something?
For me, what works is highly templated directory structure and rules that linters can quickly check and enforce. Every PR is a cookie cutter design filled in. There are only a handful of things you need and they’re all just CRUD at the end of the day. You can employ the same very few design patterns and primitives across a lot of your stack. Review is so fast, if you trust your CI to do its job and the developer to actually QA their own work.
What isn’t working is the sheer volume of PRs. Even with CODEOWNERS directing attention to only high impact changes, it is a constant distraction. I’m not sure how to handle SOC2 where we need a human reviewer on all code. Lastly, agents shit the bed when they touch spaghetti, esp when it’s sitting next to structured patterns. IME, the best way to get around this is to just rewrite features from scratch. But in this current world of instant completion and reward, it’s been challenging to convince people to commit to more ambitious lifts.
Finally, I want review more than ever. A team can’t keep up with the pace of change even if it was all perfect. I like the idea of more up front planning, but the code is ultimately what matters.
Code review is great if the code isn't a surprise, the architecture was already agreed on, the AI reviewers are already happy, and you just need a final pass.
Code review was already terrible before AI if you were slopping down weeks of work on a poor teammate to sift through.
I tell the younger engineers at my company the same thing today I said 5 and 10 years ago. Make the review easy for another engineer to do it, and they'll do it. Make it hard on them, and you won't ship shit.
Working on autonomous development with agents and no human code review. The future is human oversight, without humans in the loop. A PR centric cycle is like being in a factory and standing next to a conveyor belt of PRs. Humans can't possibly review properly. They have to take a few steps back and start looking at the entire machine, evaluating the system as a whole.
Then something is either wrong with your conception of PRs or whatever process you have been submitted to. PR is asking for feedback from people that already understand the software. It's also a learning place for anyone that is not yet familiar with the part being modified. So you have the author, the reviewers, and the learners. Which is three separate sets of people.
The PR review itself shouldn't take a lot of time (10mn is already a lot), because it's either a small change which is easily understood[0] or it's a more complex one [1] that should have a design discussion thread attached. This discussion is orthogonal to the code itself. Both should have a description good enough for the reviewer to have an idea why the change is necessary before looking at the code.
So a PR is an exchange of ideas around a real artifact, the code. If no idea is being exchanged, then it's merely a gatekeeping activity.
[0] https://cgit.freebsd.org/src/commit/?id=73b1453e6d87f9af4c47...
[1] https://cgit.freebsd.org/src/commit/?id=49a3cbc47f8e721a787e...
Their support has acknowledged the issue to us over a month ago but no fix as of now.
Anyone else experience this?
On topic, regarding these changes. I'm not sure what all changed, but I like having the status checks and merge button at the top of the page now.
Became so bad for us that we switched to an external merge queue provider (trunk.io, not affiliated)
P90 of merge queue entry _started_ dropped from up to 40! minutes to 16 seconds. And it’s the same underlying mechanism, creating a branch and running CI on it.
IIRC we unapproved it, approved without that enabled, and then manually merged it.
Companies with more than one person also have the issue that people don't communicate 100% of what's going on. This means prioritization is imperfect so they often do things other than what you'd like them to. Also you'll find different cultures which differently emphasize fixes and new work.
Hope you're not having too bad of a week whule your bug persists.
But this seems fine
If anything I think the place to invest a better UX is on reviewing code, even “my” (agent’s) own code before I ask for a review. In a world where we’re moving away from IDEs, and using terminal apps or desktop apps to code, I often don’t even see the code until it’s on GitHub. But I still cling on to some semblance of needing to review, in particular what’s important.
And yes, me one year ago would be shocked at what I just said.
I wonder, do they not notice? Do they not think we do? Do they not care about the quality of their copy? If not, why on earth would anyone conclude that they care more about the product? There is so much evidence that they do not care, this is yet another piece of data to add to the pile.
Again, it’s just so sad that a company which used to be justifiably beloved has become so miserable and incompetent.
The GitHub review page is very frustrating navigating around to different tabs.
Specifically, graphite wins on:
- I land on the page and immediately see an overview of my whole PR stack: what’s in the stack, what’s passing, what’s failing, what’s draft, unresolved and total comments. I know exactly where to spend my time. GitHub PR stacks offer no such convenience.
- Description is below stack details, then code, and I never have to switch tabs or pages. This hierarchy makes sense: top of the page is broad whole stack details, and as we go down we get more specific. GitHub forces me to jump between pages to see all of this.
- Comments in a drawer are nice. Being able to quickly scan them helps me get a sense of what’s there, who’s commenting, I can jump between them quickly.
- Approval, request, labels, etc are also logically oriented at the top near other metadata. GitHub sprawls this around the page. It’s especially rough on mobile browser.
I just checked the new overview. It’s a step up. Curious to see if it helps the PR stack experience. I’d love to fully migrate off Graphite, the non-atomic merge is truly a nightmare across a large stack, but at the end of the day I haven’t been able to give up the UX gains of managing my work on Graphite compared to GitHub.
There's not really any improvement to the core problem with PRs. You still have to read up and down the entire comments tab and then again the entire diff tab (hunting and clicking each "load more" and re-opening every "resolved" thread to find invisible replies) to reconstruct the timeline of what's happened in your head because nothing's in temporal order and there's still no interdiff.
I can’t believe people would write this as the justification. You can press PgDn on your keyboard a few times or just End once to go to the bottom. If you are on a MacBook and your keyboard doesn’t have these keys, press Fn-Down instead of PgDn and Fn-Right instead of End. When done, use the opposite keys (PgUp/Home/Fn-Up/Fn-Left). I have never once found that scrolling takes too much time.
That is the worst with their UX currently, only challenged by the stupidity of the code diff page that hide the most modified files so you can easily miss them and it is impossible to do a ctrl+f search on the page.