On Thu, Aug 27, 2009 at 1:37 PM, Mark Rowe<[email protected]> 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 ... _______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
