[Asterisk-Dev] [Rant] [long] - code style and quality

Steve Kann stevek at stevek.com
Fri May 13 09:01:08 MST 2005


Jean-Hugues ROBERT wrote:

> At 09:12 10/05/2005 +0200, steve at daviesfam.org wrote:
>
>> Heh - I used to contribute to mplayer; once upon a time they wouldn't
>> accept patches that changed indentation...
>> Steve
>

I generally think this thread should die, but I do want to point out 
that "whitespace changes" can be dangerous, if you're not really 
careful. To whit, changes in M3934
which theoretically just changed whitespace, actually removed two lines 
of code, thereby changing the operation of the code..

If you do something like this, please also use "diff -uw", to get a 
better idea of what the actual changes will be.

-SteveK

> This is very unfortunate.
>
> Fact is, the Coding Guidelines explicitly advices about avoiding
> whitespace changes.
>
> This is an issue.
>
> Over the last few years a new notion appeared, called "Refactoring".
> See http://www.c2.com/cgi/wiki?WhatIsRefactoring
>
> It's about internal changes that don't change the external behavior
> of the system. Basically: you clean the mess.
>
> To make it easier (or even possible) to refactor Asterisk's code, I
> think that a few tools are missing:
>
> 1) A non regression test platform.
> Who would dare breaking code to make it look prettier?
> With a test platform (and a test suite), you can make changes,
> including big ones, with at least some safety.
>
> 2) A computerized "styliser". Basically, if there is no tool to
> enforce the style, don't expect it to be respected. Maybe some
> variations around "indent" could do the job.
>
> 3) A semantically rich "diff/patch" solution. Where one can focus
> on the meaning of changes instead of presentation. Maybe some
> "indent+diff/patch" scheme could work.
>
> For style, an incremental solution is possible:
> When a patch is accepted, the impacted files get "indent" processed
> and it is the new indented file that is committed. Over time, all active
> parts of the system will migrate.
> To smooth the transition, the indenting tool should be released so
> that "private" patches can migrate easily too (especially if there
> is a backlog of them).
>
> I have to say that some recent comments about difficulties to have
> patches accepted are worrying. But the very pragmatic approach of
> Asterisk makes me feel that solutions will emerge as Asterisk keeps
> growing successful.
>
> Yours,
>
> JeanHuguesRobert
>
>
>
> -------------------------------------------------------------------------
> Web: http://hdl.handle.net/1030.37/1.1
> Phone: +33 (0) 4 92 27 74 17
>
> _______________________________________________
> Asterisk-Dev mailing list
> Asterisk-Dev at lists.digium.com
> http://lists.digium.com/mailman/listinfo/asterisk-dev
> To UNSUBSCRIBE or update options visit:
> http://lists.digium.com/mailman/listinfo/asterisk-dev
>




More information about the asterisk-dev mailing list