← Back to team overview

multi-touch-dev team mailing list archive

Re: Policy on formatting fixes intermixed with code changes?

 

On 03/26/2012 03:15 PM, Peter Hutterer wrote:
> On Mon, Mar 26, 2012 at 02:09:13PM -0700, Chase Douglas wrote:
>> Hi all,
>>
>> As the uTouch project is growing larger and larger, we are now starting
>> to hit issues where code formatting may stray from the unity style
>> guidelines. When we develop changes, we might see incorrect style in
>> surrounding code. Should we have a policy that requires code format
>> fixes to be contained within separate commits? This has come up as an
>> issue in a recent merge proposal, and we haven't really discussed it
>> previously.
>>
>> I have two concerns with requiring separate commits:
>>
>> * It would be nicer to always have it split out into separate commits,
>> but the burden for maintaining those commits usually means people don't
>> bother with them, even when it's within the same area of code. In other
>> words, I would rather allow for a few format fixes mixed in code changes
>> than potentially forgo format fixing contributions because of the extra
>> effort involved for trivial gain.
>>
>> * Git interactive rebase makes it very easy to split comment and code
>> changes using "git add -p" and splitting, amending, and rebasing
>> previous commits. Unfortunately, Bzr was designed with a different idea
>> of how software development should be done, and does not readily allow
>> for editing, amending, and rebasing previous commits. The best way to
>> make it work with bzr is using bzr-pipeline, but it is not as safe as
>> git rebasing (i.e. it might eat your work if you use it wrong), and it
>> doesn't handle rebasing changes out of one commit and into a previous
>> commit. I don't think it is practical to require separate commits for
>> style changes with the current bzr tools.
>>
>> If you have input to provide, please do so asap, hopefully within the
>> next day.
> 
> run an automated tool across the whole code-base. that way you only get one
> reproducible commit per package. After that, coding discipline and patch
> review should keep you there.

This particular issue was due to one merge while we were unclear what
the style was for a few places. For the most part we are in alignment,
so I don't think we need to spend time running format tools right now.
It's merely a question of how to handle fixing little one-offs that
creep in from time to time.

-- Chase


Follow ups

References