[asterisk-dev] Line length restrictions in code changes

Matt Fredrickson creslin at digium.com
Mon Mar 20 16:26:36 CDT 2017


I usually don't like to stick my hand into fires such as these (went
through this on various projects in the past and learned my lesson).
But here I go again... (oh dear oh dear):

On Fri, Mar 17, 2017 at 7:30 AM, George Joseph <gjoseph at digium.com> wrote:
>
>
> On Thu, Mar 16, 2017 at 2:35 PM, Kevin Harwell <kharwell at digium.com> wrote:
>>
>> On Thu, Mar 16, 2017 at 2:54 PM, Matthew Jordan <mjordan at digium.com>
>> wrote:
>>>
>>> <snip>
>>>
>>> Personally, I find this:
>>>
>>> if (a_good_variable_name->another_good_name &&
>>> do_something_to_it(a_good_variable_name->another_good_name)) {
>>>     // Do things
>>> }
>>
>>
>> I prefer this to the alternative as well.
>
>
> So don't indent wrapped predicates or is that just email formatting?
>
>>
>>
>>>
>>> if (a_good_variable_name->another_good_name
>>>     && do_something_to_it(a_good_variable_name->another_good_name)) {
>>>     // Do things
>>> }
>
>
> I actually prefer this one with the operators at the front but I'm fine
> either way.

This is my preference as well.  I like to see the operators at the
front in case of long line lengths that get cut off (similar to
Richard's reasoning).

>
>>>
>>>
>>> <snip>
>>>
>>> Questions than:
>>> (1) Should there even be a line length rule?
>>
>>
>> I lean toward yes just to keep things sane (e.g. keeping someone extending
>> a function definition to infinity and beyond). Also sometimes I split my
>> editor's screen vertically and have multiple code views up. The vertical
>> split reduces my column length and I either have to start scrolling or (in
>> my current setup) the line gets wrapped. Both hurt readability for me.
>
>
> This is where choice of editor or IDE comes into play.  I use Eclipse so my
> left pane is a navigator, the center pane is the editor and the right pane
> is search/outline/bookmarks.  Even with that, I have my column marker set at
> 100 which works well (for me).  Still though, there's code, mostly function
> prototypes or definitions, that make me nuts because they run well beyond
> 100 characters.  Even more annoying are header files like
> res_pjsip_session.h where both declarations and comments go out to 200
> columns.

I usually don't enforce line length at the editor level as well.
Variable flexible terminal sizes take most of the pain away of
alternative line lengths.

>> (2) If there is a line length, what is a reasonable length given some
>> of our function names? (Looking at things like
>> ast_sip_get_mwi_disable_initial_unsolicited)
>>
>> I personally think the current length of 90 is reasonable. I'll add though
>> that going a bit above this (say up to 100) in some cases should not prevent
>> code submission. For instance if a variable name is going to extend an 'if'
>> statement 4 more characters there is no reason to break that up.
>
>
> My vote is 100.

I'm good with either, stipulated with soft enforcement saved for when
line length width is abused in egregious and non-readable ways.

>> (3) Should we simply advocate for readability, with examples of what
>> is readable and what is not?
>>
>> Readability should be first and foremost, and some examples would probably
>> help with that.

I'm all for readability and good guidelines in favor of hard rules. :-)

>
>
> +1000.  I can adapt and so can my editor but having real guidelines is key.
>
>>
>>
>> --
>>
>> Kevin Harwell
>> Digium, Inc. | Software Developer
>> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
>> Check us out at: http://digium.com & http://asterisk.org
>>
>>
>> --
>> _____________________________________________________________________
>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>
>> asterisk-dev mailing list
>> To UNSUBSCRIBE or update options visit:
>>    http://lists.digium.com/mailman/listinfo/asterisk-dev
>
>
>
>
> --
> George Joseph
> Digium, Inc. | Software Developer
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - US
> Check us out at: www.digium.com & www.asterisk.org
>
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev


-- 
Matthew Fredrickson
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA



More information about the asterisk-dev mailing list