1161800 - Review Board's diffviewer is sometimes noisy

Status: RESOLVED WONTFIX

(MozReview Graveyard :: General, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Filing this on behalf of jrmuizel. I'm not even sure this is something that can be fixed on MozReview's side - I actually suspect this is more of a Review Board core issue.

Check out the patch in bug 1150549: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1150549&attachment=8600949

This patch was also uploaded to Review Board here: https://reviewboard.mozilla.org/r/8175/diff/

In gfx/layers/composite/TiledContentHost.cpp, Review Board is marking a lot of stuff as "moved", but that's of little utility seeing as this whole file was refactored. It actually adds a bit of noise, according to jrmuizel - especially since the lines that RB skips as unchanged are closing braces and void, which isn't particularly helpful.

Not sure I've captured the issue properly here. Maybe jrmuizel can articulate it better.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I'm curious to know if bug 1161791 would fix this...

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Here's the github version:
https://github.com/jrmuizel/diff-tests/commit/ef04239ece58c0431b7f44c36b067a8fe45a278e?diff=split

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #1)
> I'm curious to know if bug 1161791 would fix this...

I'm guessing not.

Comment 4

[Mark Côté [:mcote]]

I get what you're saying, but I don't know if we can have both the fancy and generally helpful "nonstandard" diff features (moved code, indentations, etc.) and improve the readability of refactors like this.  I would advise making the diffs a little simpler if possible, and/or looking at the raw diff when things appear confusing.