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

Pull Request File Tree Feedback

github.com

Post reply on HN
Print view View original

61–70 of 77 posts

Re: Pull Request File Tree Feedback

#61

Post by voidfunc »

OliverGilan wrote: post #11

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.

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.

Re: Pull Request File Tree Feedback

#62

Post by mushyhammer »

OJFord wrote: post #31
svnpenn wrote: post #19

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.

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

> seems better

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.

Re: Pull Request File Tree Feedback

#63

Post by MitchellCash »

petepete wrote: post #59
inetknght wrote: post #21

Earlier quoted context omitted.

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

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.

Re: Pull Request File Tree Feedback

#64

Post by OJFord »

mushyhammer wrote: post #62
OJFord wrote: post #31

Earlier quoted context omitted.

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

> seems better 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?

Re: Pull Request File Tree Feedback

#65

Post by formerly_proven »

eloisius wrote: post #17

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…

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.

Re: Pull Request File Tree Feedback

#66

Post by alkonaut »

RyJones wrote: post #48
alkonaut wrote: post #9

Earlier quoted context omitted.

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.

ADO will go away once all of the features are migrated to GitHub.

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.

Re: Pull Request File Tree Feedback

#67

Post by isbadawi »

lachenmayer wrote: post #52
eloisius wrote: post #17

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…

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.

There's an item on their roadmap to allow commenting on unchanged lines: https://github.com/github/roadmap/issues/456

Re: Pull Request File Tree Feedback

#68

Post by petepete »

MitchellCash wrote: post #63
petepete wrote: post #59

Earlier quoted context omitted.

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.

A toggle in GitHub settings to automatically collapse `*.lock` files would be a massive step in the right direction.

Re: Pull Request File Tree Feedback

#69

Post by eloisius »

other_herbert wrote: post #28
croddin wrote: post #26

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

Just wait till you hit . On your keyboard

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.

Re: Pull Request File Tree Feedback

#70

Post by radicality »

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.
Post reply on HN