multi-touch-dev team mailing list archive

[Peter Hutterer <peter.hutterer@xxxxxxxxx>]

On Tue, Mar 27, 2012 at 04:23:01PM -0300, Daniel d'Andrada wrote:
> On 27/03/12 13:38, Chase Douglas wrote:
> >On 03/27/2012 06:10 AM, Daniel d'Andrada wrote:
> >>On 26/03/12 18:09, 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 suggest only fixing coding style in the lines your commit is going
> >>change anyway and not doing changes that solely fix coding style at all,
> >>even if they come as a separate commit. Unless something really ugly or
> >>big has happened (e.g. an entire file is using tabs instead of spaces).
> >Why not?
> Pollutes unnecessarily the commit history. Commits that fix coding
> style will get in front of the commits that have added or modified
> the logic. So you'll have to dig below coding style commits to find
> the real commits containing meaningful information about the added
> or modified logic.
> 
> In the end it comes to what the developers of a project value more:
> having all the code with correct coding style or having uncluttered,
> more meaningful, commit history. I prefer the latter.

speaking as someone who's worked on a project with no real coding style for
years the mess you end up with is unbearable. You'll end up writing code
where two hunks in the same function may have different indentation styles
because no-one ever bit the bullet and re-indented the whole project.

as for polluting the history: we ran indent over the whole X server a few
weeks ago yet if I run git blame on a file that got completely rewritten,
the number of lines that show up from that indent is reasonably small. This
is after all something the tools can and should help with.

$> wc -l xkb/xkb.c
6864 xkb/xkb.c
$> git blame xkb/xkb.c | grep "9838b703" | wc -l   
4597
$> git blame -w xkb/xkb.c | grep "9838b703" | wc -l
532

So once I tell git to ignore whitespace changes, only 532 out of 4597
changed lines are attributed to that commit. A good tradeoff.

Cheers,
  Peter