Hacker News Reader

Comment by bsaul | Hacker News Reader

there’s so many low hanging fruits for improving the quality of diff viewing. The worst code reviews are often the ones where code get refactored, leading to piles of delete / create lines that are just code being moved or slightly renamed.

One very simple approach would be better git integration with the IDE, helping build commit that make sense, where a set of changes could easily be commented by the author as they’re performing the edits, then keep improving from there.

View on HN

This is a concept I almost pursed for my PhD over a decade ago. I’m still surprised no one has done this.

The bigger picture: context is often missing for anything complicated, be it software or a new law. Yet many hands touch and retouch the underlying material over time. If you could capture _how_ something was built, and had enough insight into the larger process to sample some of the _why_, then you could both know what changed together and what potentially impacted the final decisions. This would result in (hypothetically) tremendous gains for anyone working on or joining a project that’s bigger than can fit in the mind of one person.

The PR history can answer the why, and if you can anticipate someone will ask why because you know it is edge-condition spaghetti then you can document it right in a comment.

This is a pretty myopic stance that says everything is fine. It’s not. PR histories can be gigantic, interwoven, doesn’t tell you how and only sometimes tells you the why—-usually at a very fine grain of detail. Saying that you can “document it right” is like saying “code it well in the first place.” Much of the time there aren’t great comments or PRs, and what there is ends up assuming a huge amount of knowledge about the why/how. If you’re an outsider coming in, we should be able to do way better to help.

JetBrains has a code review platform that lets you do diffs in your IDE. The idea is incredibly appealing but for some reason I found it not so great in practice. Something about viewing the diffs in a different UI gets me in a different frame of mind. I found it hard to get into review mode in my IDE.

There are advantages of course, being able to jump around and get the context at will.

You can almost get there if you commit often, and structure your commits such that all the trivial stuff are done in their own commits; for instance, if you introduce an if, you add the if and end in one commit and do all the re-indenting of existing lines in another.

They measure the "quality" of review based on time spent looking at the code which suggests to me they have absolutely no idea why they are making people do code review at all.

This is an especially bad metric because

1) We have good data that after about 60 minutes of reviewing we start to lose the ability to find more issues.

2) It incentives making bigger and harder to review changes so that people spend more time looking at the code.

You seem to suggest that the team is trying to increase eyeball time metric, but that isn't what the post said. The post said that they wanted to decreases the time to review metric, but without changing the eye ball time metric.

Reply on news.ycombinator.com