[asterisk-dev] Line length restrictions in code changes

[Joshua Colp]

On Thu, Mar 16, 2017, at 04:54 PM, Matthew Jordan wrote:
> 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
> }

Agreed.

> 
> 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
> }
> 

<snip>

> 
> 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?

While we've had a line length rule I don't have any indication of it in
my text editor or count. I personally value readability above all. I
generally start a new line where it feels reasonable and flows best. I
don't think we need to enforce (or have) a line length rule and think
that people could naturally settle on what seems best when they are
working on the code in question.

Cheers,

-- 
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org