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

Unified versus Split Diff

matklad.github.io

Post reply on HN
Print view View original

11–20 of 184 posts

Re: Unified versus Split Diff

#11

Post by friendzis »

lelanthran wrote: post #7

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

LHS shows current code, without any modifications. By showing full context in one view (with code browsing capabilities!) and changes in the other, the author is able to browse the code and see what changes, if any, are applied in a particular context.

In my understanding the author wants to flip the diff around: instead of looking at changes themselves, the author wants to look at code and see if there are any associated changes.

The article is light on detail, but my guess would be that author wants to look at code and browse to implementation/callsite to check if appropriate changes are there.

Re: Unified versus Split Diff

#12

Post by klauserc »

Ooooh, I need to try this!

When working on a PR myself, I frequently avoid doing small, incremental commits because I find the subtle "these lines changed" annotations in IDEs extremely useful. It helps me find the locations in code that are relevant to my work.

I wish there was a way to configure e.g., IntelliJ to always show these markers relative to `main` instead of the last commit.

Re: Unified versus Split Diff

#14

Post by klauserc »

lelanthran wrote: post #7

I'm probably missing something here: author says that the split diff doesn't work for him, but doesn't say why. His ideal diff is pretty much the same as a split-diff but with redundant context removed (context is only on the left, not on both left and right). What utility does he get out of removing redundant context on the RHS of the pane?

The author writes

> I need to run tests, use goto definition and other editor navigation features, apply local changes to check if some things could have been written differently, look at the wider context to notice things that should have been changed, and in general notice anything that might be not quite right with the codebase, irrespective of the historical path to the current state of the code.

The editors/IDEs I've used usually only offer rudimentary support in diffs (goto defn, but only in the same file; maybe auto-completion, but usually not for newly added items; no refactoring functionality)

Re: Unified versus Split Diff

#16

Post by sam_lowry_ »

phillipcarter wrote: post #4
dylukes wrote: post #3

Earlier quoted context omitted.

> In my book a general code review is simple sanity check by a second pair of eyes, which can result in suggestions to use different API or use an API slightly differently. This is an impoverished view of code review. Code review is a principal mechanism for reducing individual code ownership, for propagating conventions, and for skill transfer. A good code review starts with a good PR: one that outlines what its goa…

As a counterpoint, individual code ownership can be a fantastic model too. It lets engineers specialize and takes advantage of social systems that form naturally anyways. I’ve not personally seen group ownership work well, and in practice, it’s still an individual who knows a given area the best.

> reducing individual code ownership

I am now working in an organization that is set up to reduce code ownership, and they struggle to attract talent, although pay is good and work is fulfilling.

How do they do reduce individual code ownership? Horizontal integration. Developers code, analysts design DB structures (at least nominally), project managers set up meetings. Different silos exist for CICD, cloud roles, core teams.

There are vetting committees everywhere that have the last say on the libraries used and the nitty-gritty details of REST APIs and naming.

It exhilarating to start projects, then see them degrade inevitably into corporate monstrosities.

Re: Unified versus Split Diff

#17

Post by difosfor »

I’m surprised how we’re all still using line based diffs and even seemingly stupid ones at that. But the tales I hear from across the pond of paid git alternatives which diff based on understanding the programming language seem to be pretty bad as well. Though for other reasons?

Re: Unified versus Split Diff

#19

Post by lvncelot »

friendzis wrote: post #2

> For a large change, I don’t want to do a “diff review”, I want to do a proper code review of a codebase at a particular instant in time, paying specific attention to the recently changed areas, obviously every team and ticket is different, but IMO unless the person doing the review is some sort of principal engineer mostly responsible for the code at large, this does not align with I would personally consider a cod…

I agree about what to expect from a code review - but for me, certain ways of working need to be in place for this to function properly.

A code review is the "last line of defense" (well, disregarding CI), but in the teams I've worked in, that meant that the general idea of what the PR is introducing has been discussed by multiple people at that point. (This could be either a pairing session, an in-depth explanation, or just a coffee chat, depending on the complexity) This way the PR isn't about reviewing the general strategy (splitting off a new module, introducing a huge dependency, reworking the API surface), but just about reviewing the tactics used to implement that strategy.

Without the communication beforehand, doing only sanity checks on parts of the code in isolation does run the risk of fracturing code ownership. ("What's that module doing?" "Beats me, ask bob")

That all notwithstanding, I like the author's idea of what a diff view should look like, regardless of the "mode" of reviewing.

Post reply on HN