Hacker News Reader

Comment by inetknght | Hacker News Reader

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

View on HN

Might be worth making a bounty for it in refined github[0], similar things have been implemented in the past[1]

[0] https://github.com/refined-github/refined-github

[1] https://github.com/refined-github/refined-github/issues/2151

wtf is refined github? A browser extension? Nah, I value my browser more than that

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.

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.

Better to just find a dev-first platform instead.

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.

There's no denying the appeal of underdogs like SirHat :)

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.

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 have no idea why we include these in code reviews tbh

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.

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.

Reply on news.ycombinator.com