[webkit-dev] Reformatting-only patches being applied to trunk
staikos at kde.org
Sat Jul 25 05:25:48 PDT 2009
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
>> 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
> 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.
Torch Mobile Inc.
More information about the webkit-dev