top | item 30647047

Pull Request File Tree Feedback

103 points| isbadawi | 4 years ago |github.com

73 comments

order

eloisius|4 years ago

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?

inetknght|4 years ago

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

andrewf|4 years ago

"Our metrics tell us that with these UI changes, people are approving more PRs and it's taking them less time!", probably.

88913527|4 years ago

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.

thombles|4 years ago

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.

lachenmayer|4 years ago

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.

humanwhosits|4 years ago

Hiding of the large files has tripped me up multiple times. I've starting having to look at the diffs outside github's UI

formerly_proven|4 years ago

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.

ydnaclementine|4 years ago

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

88913527|4 years ago

I often use `t` to quickly search for a file by name. It's got reasonably good matching: querying for "foo.svg" will show a match for "foobarbaz.svg".

gfody|4 years ago

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.

politelemon|4 years ago

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.

wdb|4 years ago

Gitlab and big PR aren't a great experience. Impossible to scroll -- it's so slow

mrrsm|4 years ago

If only Bitbucket Cloud would play catchup now. So many times I read about a bitbucket feature to only find out it is server only and we can't use it.

OliverGilan|4 years ago

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.

carstenhag|4 years ago

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

oneepic|4 years ago

+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.

voidfunc|4 years ago

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.

tomasreimers|4 years ago

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 :)

brainbag|4 years ago

I've been looking for something like this for weeks. Seems fantastic. How do I get that invite?

jameslao|4 years ago

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.

[0] https://www.crocodile.dev/

eloisius|4 years ago

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.

noname120|4 years ago

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.

[1] https://www.octotree.io/

dandigangi|4 years ago

One of the few things worth actually stealing from BitBucket. Lol

alkonaut|4 years ago

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.

marceloabsousa|4 years ago

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).

radicality|4 years ago

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.

jameslao|4 years ago

I'm also working on a code review app called Crocodile that addresses GitHub's shortcomings. Signups are open and feedback is always appreciated.

https://www.crocodile.dev

svnpenn|4 years ago

What's ironic is that discussion pages, like the one linked here, are broken on mobile. Maybe they should focus on that first.

Also, I hate repos that convert issues to discussions. Might as well close the issue, as discussion is usually a graveyard.

OJFord|4 years ago

Why is closing better? Either means 'maintainer won't do anything' (beyond perhaps charitably helping you out)

'Moved to discussion' seems better to me than 'closed; tagged question'.

BugsJustFindMe|4 years ago

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.

croddin|4 years ago

That seems very useful for large pull requests. GitHub is starting to look more and more like VS Code.

other_herbert|4 years ago

Just wait till you hit . On your keyboard

eat_veggies|4 years ago

I've been using the Octo Tree extension for this, but it's so great to have it built-in !

lh15|4 years ago

This is why I’ve enjoyed using Bitbucket more than GitHub. Nice to see github adding it

duxup|4 years ago

I like it as a quick glance on generally what is up.

I don’t like scrolling until I see “holy cow wat”.