On 25-Jul-09, at 6:46 AM, Maciej Stachowiak wrote:
On Jul 25, 2009, at 2:08 AM, Oliver Hunt wrote:
I've just noticed that there have been a few purely style related
patches being landed in the tree recently, I don't believe these
are a good idea and that any further reformatting only patches be
rejected.
Historically we have avoided purely style related changes as they
impact our ability to track code changes efficiently and make
patch merging more complex. The general approach to code cleanup
is to clean up regions of code as we work in them -- this means
that more-or-less the only code effected by reformatting is code
that was being modified anyway.
Does anyone disagree with this approach?
My opinion:
I think pure style/formatting changes should be separated from
substantive behavior changes. I like it when style changes come as
separate patches but in coordination with other patches that make
substantive changes to the area. It's true that we've never made a
comprehensive effort to reformat all the code to match style
guidelines. But we have had some pure style cleanup changes to
sections of the code, and have made some big changes like the one-
class-per-file split long ago, as well as renaming our top-level
namespaces.
I would say for files that are already close to correct style,
minor changes to fix the last few issues are likely a good idea.
The cost is low and the benefit of making our code more consistent
is worthwhile.
We prefer to fix style gradually, but doing some systematically is
ok. If some of these changes end up being big, it may be worth
broader discussion. Large-scale reformatting is something that has
project-wide impact.
More generally though, I would say now is a good time to do code
cleanup and refactoring. It seems like a good time for some
consolidation and entropy removal, after a lengthy period of
aggressive feature development.
I agree with this. I don't like reformatting changes at all
unless the code is really truly illegible, but if they're done, they
should be done separately from changes that actually have substance.
--
George Staikos
Torch Mobile Inc.
http://www.torchmobile.com/
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
