I’d settle for them making PRs as useful as they were in 2015, before they messed up some of the most basic functionality: showing the diff, and showing review comments. They hide big diffs behind a “load more” link, and as a result people often fail to code review the most substantial part of a change because they scan right past it, thinking it’s a removed file or binary or something. Then, once you submit a review, they only show 10 comments. In the middle, there’s an easy-to-miss “load more comments” button.
These are the two most fundamental features of a PR. How could they decide so few as 10 is the right number of comments?
> They hide big diffs behind a “load more” link, and as a result people often fail to code review the most substantial part of a change because they scan right past it, thinking it’s a removed file or binary or something.
This. Every PR I have to do ctrl-F "load diff" and then immediately click on _all_ of the diffs. It's !@#$ing annoying.
I've also lost comments when the comment is part of a review and pushed to the PR after the the PR has been merged by someone else.
An extension shouldn't be needed for a dev-centric service like GH to be usable. This is the wrong way to fight bad UX, as it's ridiculous to make installation of a potential security vulnerability of an addon necessary to make a git-frontend website work well.
Perhaps you are more fortunate than me, but in my career, there have been times where I couldn't dictate the platform used. This is actually the case for many people - I daresay the majority - who work in the software industry. So while I understand (and of course agree with) the sentiment, it's rather trite. Plugins are there for the rest of us.
Seriously. If they’re too busy making sketchy copyright decisions with their AI code generation to bother with basic usable UI, they don’t deserve to be the de facto home of open source. I’ve been giving sourcehut a try, and the simplicity is refreshing.
Especially for GitHub.com cookies and access. With over 50k users, this has to be a prime target for phishing/other attacks against the maintainers to publish a malicious update to the extension. Think passively stealing creds and source code.
For the stuff I work on I prefer the big diffs to be hidden. It's nearly always yarn.lock and I have no desire to see that monstrosity in all its glory.
I think we can get the best of both worlds by hiding lock files by default and always showing any actual code file diffs. Performance may be a factor for very large diffs, but this would actually be my preference to how the default behaviour would work as I agree with you and the parent post.
Not saying I review dependency locks as well as I should, but one reason to is to prevent supply chain attacks. E.g. making a typo that installs a malicious package. I recently saw a $60k beast of a machine with 64 cores get pwnd. We all wondered why “-bash” was burning 48 cores of CPU until I attached strace to it and we saw JSON RPC messages indicative of crypto mining. Everyone with access to the machine is trustworthy, but someone may have typo’d a pip install or something like that.
Maybe they're trying to nudge us towards smaller PRs. Many reviewers gloss over 300+ line changes in a single file. Approved with a "LGTM" and no further comment, but perhaps that's more a cultural issue with the team than anything.
Perhaps, but github's PR UI goes to utterly atrocious if you try to do something along the lines of "a series of small atomic changes". In my recent reviewing experience, anything involving multiple commits in a single PR results in the diff more or less lying to you: you get diffs of something, but it's not clear what, and even less clear how to get what I want to actually diff.
I think the evidence is pretty clear that the most effective development process for large codebases is to look at changes as effectively a series of patches applied to head-of-trunk (and those patches may evolve during review)... and github seems to almost go out of its way to prevent you from thinking about PRs as if they were patches.
> github seems to almost go out of its way to prevent you from thinking about PRs as if they were patches
Those who take care to write meaningful per-commit descriptions are actively punished by the eager collapsing. Either reviewers have to (1) spot the ellipsis (2) click on it for each individual commit, or the submitter has to copy-paste all of their commit information into the PR description.
> anything involving multiple commits in a single PR results in the diff more or less lying to you: you get diffs of something, but it's not clear what, and even less clear how to get what I want to actually diff.
I'm trying to work on solving this and I have a first iteration of it which I wouldn't mind getting feedback on. If you look at the following PR:
you can easily identify which commit does what. For example, if you select the first two commits and filter by them, the PR tree will show you the files that were changed by those commits and if you click on the commit in the tree, you can view the diff for that commit.
In the future, I want to support what you described and make it very easy to diff any revision.
While smaller PRs are better when practical/possible, the current Github design doesn't encourage them at all. If anything, it makes it too easy to submit giant ones - a bunch of the changes may get hidden anyway, making it look less intimidating than it should be!
Additionally, the notification email links to "View it on GitHub" don't reliably cause the relevant parts of the page to expand so you end up wading through a huge PR expanding things at random until you find the message.
This is by far the most infuriating thing about the PR UI, along with the fact that you can't comment anywhere outside of the "patch", meaning you can't point out anything that is more than a couple of lines away.
It's still not as bad as BitBucket which makes it basically impossible to read commit messages in a pull request. Is that a good level to be at? Probably not.
In the screenshot in the post it looks like the navigation tree is crammed into the 1280px wide grid, but that's not the case - enabling the feature preview makes the page full-width. So it doesn't make the diff area unreadable.
Not sure if this is well known, but press period `.` when viewing a PR, repo, or file and github will send you to a in-browser visual code editor. Able to make commits in there too, perfect for [nit] comments
if you press it from a PR you'll get the vscode PR viewer which is way nicer than Github's (IMO, especially for larger PRs) the only thing missing is the ability to switch between changes from last commit, last review, etc.
I didn't know about it! I primary use a corporate hosted instance GitHub (so we often opt in to new features late), and I don't follow a lot of tech news on social media.
Bitbucket Server and Gitlab have this feature and it's quite useful for very large pull requests as you can easily see the folder structure of the file you're reviewing, for that bit extra bit of visual context. Bitbucket's search box is slightly better because it also does a code search within the PR, it helps you quickly find specific words (say, a class name) across all the changed files. Gitlab's only does a file name filter.
Though relatively late, I am glad it's coming to Github, more people can benefit from this kind of pull request presentation.
Funny but Azure Repos actually has had this feature for a while. When I first joined Microsoft I was shocked that all our teams used Azure Repos instead of GitHub considering we own GitHub but as I've used Repos more and more I've actually come to like it more than GitHub itself. A lot of the UI is cleaner and more intuitive than GitHub to me now, maybe just from using it a lot.
Agree, I really prefer DevOps' PR experience. I'm just missing the easy GitHub ci Integrations (3rd party checks & bots that post comments for example). It probably also exists for DevOps, but I never came across of it
+1. I had the same experience joining Microsoft. I wasn't super thrilled by Azure at first, but was surprised by how much functionality it had. Fast forward a couple years later, I moved to another big tech company using Github instead, which had setup integrations with a bunch of different external tools (and this one was no slouch when it comes to engineering tooling). I can't tell you how much I missed Azure, even though I've had my issues with MSFT.
Azure DevOps is a very solid product in general. That said, the whole work items UI is terrible and inferior to the GitHub issue’s system. Too complex.
Hi! I am one of the authors of https://graphite.dev, we are basically a really fancy client to GH that lets you review others PRs without making them change their workflow at all (posts everything to GH etc)
We've had a file tree for some time now (along with some of the other feedback I'm seeing in this thread, large diffs etc).
If anyone wants to give it a spin, happy to give you an invite :)
If anyone is interested, I've been building a code review tool called Crocodile[0] that lets you review GitHub PRs. It has a similar file browser to the left plus floating comments, threaded discussions, and more.
Just casually following this thread, this is the third SaaS I've seen offering an alternative code review tool to supplement GitHub. It's either sorely needed to make up for how bad GitHub is, or it's true that we engineers can't help but makes tools for people exactly like ourselves.
Probably a mix of both. Many of the founders of code review tools came from companies that had internal code review tools (Microsoft has CodeFlow, Google has Critique, Facebook has Phabricator) and they missed those tools after leaving those companies.
I'm currently using Octotree[1]. It has more features than this proposed GitHub implementation. Namely, files have icons, count of added/removed lines is displayed inline, comments are displayed inline (super easy to jump from comment to comment), etc. For now I'll keep using Octotree but I'm curious of the direction this implementation will take.
Since GitHub is owned by Microsoft and their other product has had this, I’m guessing it’s yet another copy-over like GitHub Actions was from Azure Pipelines.
Not sure how long it’s been in AzureDevOps but it could have been inspired from GitLab there I suppose.
I wouldn't mind if they converged the products tbh, but there are quite a lot of things yet to migrate I think. An extensive process thing with easy ways to hook in for doing processes and reports would be much better than what ADO does now with an extremely complex model that you invariably still need to customize with extensions and hooks.
If you don't want just a tree but also a semantic diff that tells you what types and methods where actually changed check out Reviewpad (http://reviewpad.com).
After many years at Facebook I’ve recently switched jobs and have to use GitHub now for work. Compared to Phabricator (FB’s code review tool), the GitHub code review (and code merge) processes seems extremely basic and clunky.
That looks cool, I put my work and personal emails on the waitlist! GitHub review is just… awful
Side-note: I took a look at the ‘About’ page thinking ‘Hey, they said ex-FB, extremely unlikely but maybe i know someone’. Looking at your username I’m guessing you’re jacob gold and worked on messaging infra in NY. I used to work on Iris! Small world.
That’s why they do it, but what they’re doing is essentially rejecting the request. If it’s a question, sure, but if it’s a feature request or worse yet a bug report then moving to discussions is bs.
Sure, but isn't reject + allowing discussion (e.g. from other users, or maybe a maintainer will chip in) better than reject + nothing, a closed undiscoverable issue or worse locked?
Display performance switching between files in Safari is terrible on very large PRs. I don't know what it could be doing. Maybe something to do with text reflow? But it's almost unusable.
Is this why the UI had become so slow and bloated feeling? A massive IDE in the browser? I hate how every tech company finally arrives at a stage where they do everything to keep you on the platform. I won’t even be surprised if some time before 2030 the phase out Git entirely and rename it Microsoft Visual VCS, only accessible by writing code in their Electron text editor or directly in the browser.
These are the two most fundamental features of a PR. How could they decide so few as 10 is the right number of comments?