watch out for badly-formed patches

Parker Coates parker.coates at kdemail.net
Tue Apr 27 00:24:23 BST 2010


On Mon, Apr 26, 2010 at 19:03, Chani wrote:
> just wanted to warn you guys, what you see in kompare *might* not be what's
> really in a patch (if the patch is badly formed). since a lot of us are
> reviewing patches on reviewboard these days, and actually *applying* a bad
> patch would set off warnings, it's probably not such a big deal... but if
> you're reading .diff files, doublecheck the plain text before approving things.

On a related note, be warned that ReviewBoard hides certain kinds of
whitespace issues. If the contributor of a patch wasn't careful with
his/her indentation, they may have generated a lot of junk lines that
just replaced tabs with spaces or vice versa. Because ReviewBoard
seems to work only with spaces internally, it will ignore such changes
entirely, not even marking the lines as changed.

So before clicking "Ship It!", please download a copy of the patch and
quickly scan it for anything out of the ordinary, especially if the
patch author is a new contributor who may not be familiar with style
and formatting rules. While not a huge deal, this has come up a few
times for us in KDEGames where a patch went through several rounds of
review/update before anyone noticed that the indentation was all
wrong.

Parker




More information about the kde-core-devel mailing list