Hacker News Reader

Comment by seedie | Hacker News Reader

FWIW, whitespace changes can easily be filtered out in Github's PR view.

It's a mistake to filter out whitespace changes on Python diffs. These days it's best use `Black --check` or similar in CI to make sure they've matched your whitespace settings, to minimise these changes.

Sure. The link above is in "hide whitespace" mode (`?w=1`), and there are still visible whitespace changes (new or removed whitespace lines).

Like 3 of them. Is that problematic enough that you cannot review the rest of the code? My eye just slides over them.

I am working on a GitHub pull request viewer that displays changes using a semantic diff, and therefore has some more advanced whitespace handling behavior than just ignoring leading or trailing whitespace. I tried it with this PR:

https://app.semanticdiff.com/django-money/django-money/pull/...

It doesn't make a huge difference, but it filters out changes like the added line break in "if value: value = str(value)" nicely. I haven't announced the project yet, but maybe someone will find it useful :-)

I don't think this was the case in 2016 though.

They added it to the UI in 2018 but you could do it via URL since 2011.

> A diff view with reduced white space has been available since 2011 by adding ?w=1 to the URL.

https://github.blog/2018-05-01-ignore-white-space-in-code-re...

Also, Git had it since forever, and since GitHub's diff view isn't particularly convenient for browsing multi-commit PRs you usually review the changes using Git anyway.

That said, I'd ask the contributor to tidy up the branch first. It's kinda disrespecting to ask others to review branches in such state.

Reply on news.ycombinator.com