[webkit-dev] Whitespace changes

[Dirk Pranke]

On Thu, Aug 27, 2009 at 1:37 PM, Mark Rowe<mrowe at apple.com> wrote:
>> +1
>>
>> I see little point in having coding standards if you don't encourage
>> people to use them. There is enough churn in the tree that there are
>> already a large number of changes to skip over, so skipping over
>> reformatting (even whitespace-only) seems like a small burden to me.
>
> There's a difference between enforcing our coding style on new code, and
> retroactively applying our new style to the hundreds of thousands of lines
> of code that already exist.  Your comment conflates these two issues.
> No-one is going to disagree that we should enforce the coding style on new
> code.  That would be stupid.  However it is clear that applying our current
> coding style to existing code is a much less clear-cut issue.  We wouldn't
> be having this discussion if it were as obvious as your comment suggests.

Yes, there is a difference. However, speaking as someone new to the
codebase, who is primarily fixing small bugs at this point, all of the
code already exists, and the fact that different chunks of code is
written in different styles makes my life harder than it would be
otherwise (not to mention that Chromium and WebKit use slightly
different coding styles as well). I suspect that 80%+ of the CLs are
in fact in the same camp that I'm in, since maintenance dwarfs new
code.

Moreover, not bringing existing code into compliance with new coding
standards dramatically undermines the usability, applicability, and
relevance of coding standards whatsoever. At a previous company, it
was practically impossible to impose coding standards even though we
all knew it would've been a good idea. People were too afraid to
retrofit code, and as a result, saw no point in trying to improve new
code, either, since you still wouldn't have a consistent codebase. The
one drove the other, and is arguably more logical than the "new code
only" practice.

Last, not cleaning up old code separately from making substantive
changes causes you to confuse the two changes (why did this line
change - is it part of the patch, or just cleanup)? And I think we
would all agree that you want to separate those two changes out as a
result - this is just the extreme of that line of reasoning.

So, while the right answer may not be "obvious", in my opinion it is
clear once the issues have been properly considered. In my experience,
people view these infrastructure changes as if they will be extremely
painful and hence aren't worth doing. However, they usually end up
being painful for a day or a week, and then recede into memory. SCM
system changes are a great example of this. The Changelog thread is
another ... and so we go on, getting paper cuts one or more times a
day until it adds up to a thousand and Hyatt snaps :)

As to the trailing whitespace issue, I'm actually with you on this
one; I've never seen that mentioned or considered as a coding style
guideline before this project, and am continually getting tripped up
by it on Chromium's presubmit hooks. Why do we think that this
particular rule is a good one? I much prefer just using 'diff -b' :)

-- Dirk

PS. Special thanks to anyone who can tell me how to configure Vim,
Emacs, Visual Studio, and XCode how to automatically strip those
spaces so that I don't get tripped up by this ...