[webkit-dev] Reformatting-only patches being applied to trunk

[George Staikos]

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/