[asterisk-dev] Line length restrictions in code changes

[Matthew Jordan]

Warning: bike shedding.

On several code reviews, comments have been left along with a -1 on
the review - indicating that it is not acceptable for merging - due to
line length restrictions. While this is technically correct per our
coding guidelines, it doesn't always result in the most readable code.

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
}

To be preferable to:

if (a_good_variable_name->another_good_name
    && do_something_to_it(a_good_variable_name->another_good_name)) {
    // Do things
}

This is typically because the indentation of the second part of the
boolean test lines up with the code that is to be executed if both
conditions is true, which is more visually jarring than just reading
the first line.

I'll grant this is probably a personal preference, but we have
certainly opted for the former over the latter in many places in the
code.

While I know that we aren't all sitting around with dual screens, we
also aren't living in 1979 with dumb terminals. The line length
restriction of 90 columns seems reasonable until you end up with long
function names, which - since we live in a land of void * - is
generally something that we should encourage. I'd like to say that
some common sense about when to apply the line length rule and when
not to would "work for us", but that doesn't appear to be the case.

Questions than:
(1) Should there even be a line length rule?
(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)
(3) Should we simply advocate for readability, with examples of what
is readable and what is not?

Matt

-- 
Matthew Jordan
Digium, Inc. | CTO
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org