apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vlad Rozov <v.ro...@datatorrent.com>
Subject Re: Code style: Line length enforcement
Date Wed, 02 Dec 2015 16:53:47 GMT
It is highly subjective where wrapping increase or decrease readability 
and where it is OK to skip reading code that does not appear on a screen 
or does not fit editor window. Once it becomes recommendation, the limit 
will not be enforced by an automated tool and we purely rely on pull 
request reviewers/commiters to check for proper wrapping. IMO, reviewers 
should focus more on code logic that can't be checked by a tool rather 
than wasting time on whether or not wrapping follows recommendation and 
the later can be delegated to the checkstyle.

Thank you,

Vlad

On 12/2/15 08:15, Siyuan Hua wrote:
> +1 for recommendation
>
> Hard limit will make code look ugly and actually decrease the readability.
> For example, break method signature/for loop to multiple lines
> But chain method call is good to be broken into lines
>
>
>
> On Wed, Dec 2, 2015 at 8:01 AM, David Yan <david@datatorrent.com> wrote:
>
>> +1 for recommendation over enforcement
>>
>> On Wed, Dec 2, 2015 at 7:58 AM, Munagala Ramanath <ram@datatorrent.com>
>> wrote:
>>
>>> Recommendation good, enforcement bad.
>>>
>>> Code reviewers can also "strongly recommend" on a case-by-case basis.
>>>
>>> Ram
>>>
>>> On Wed, Dec 2, 2015 at 7:14 AM, Sandeep Deshmukh <
>> sandeep@datatorrent.com>
>>> wrote:
>>>
>>>> +1 for having 120 length as a recommendation.
>>>>
>>>> Regards,
>>>> Sandeep
>>>>
>>>> On Wed, Dec 2, 2015 at 12:54 PM, Shubham Pathak <
>> shubham@datatorrent.com
>>>> wrote:
>>>>
>>>>> +1 for having 120 length as a recommendation . Enforcement would
>>>> compromise
>>>>> on readability.
>>>>>
>>>>> On Wed, Dec 2, 2015 at 11:56 AM, Chandni Singh <
>>> chandni@datatorrent.com>
>>>>> wrote:
>>>>>
>>>>>> I agree with Thomas. Having 120 length should be a recommendation
>> not
>>>> an
>>>>>> enforcement.
>>>>>>
>>>>>> Chandni
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Dec 1, 2015 at 10:13 PM, Priyanka Gugale <
>>>>> priyanka@datatorrent.com
>>>>>> wrote:
>>>>>>
>>>>>>> +1 to have fixed length.
>>>>>>> We need to check if there any way to help editors split the big
>>>> strings
>>>>>> in
>>>>>>> better way.
>>>>>>> Or for such exceptions, coder can format the part manually,
>>> adhering
>>>> to
>>>>>> the
>>>>>>> length restriction and giving better readability.
>>>>>>>
>>>>>>> -Priyanka
>>>>>>>
>>>>>>> On Wed, Dec 2, 2015 at 11:36 AM, Pradeep A. Dalvi <
>>>>>> apache@pradeepdalvi.com
>>>>>>> wrote:
>>>>>>>
>>>>>>>> -1 for 120 hard stop. There should be guideline to follow
120
>>> line
>>>>>>> length,
>>>>>>>> not an enforcement.
>>>>>>>> Few characters ahead of 120 limit shall be allowed, if
>> otherwise
>>>>>>>> compromises readability.
>>>>>>>>
>>>>>>>> On Wed, Dec 2, 2015 at 11:20 AM, Timothy Farkas <
>>>> tim@datatorrent.com
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> +1 to keep 120 line length. In my opinion it improves
>>> readability
>>>>>>> because
>>>>>>>>> it allows you to read code by only scrolling up and down.
If
>>> you
>>>>> mix
>>>>>>>> having
>>>>>>>>> to scroll up, down, left, and right into the mix it can
>> become
>>>>>>> difficult
>>>>>>>> to
>>>>>>>>> read code.
>>>>>>>>>
>>>>>>>>> On Tue, Dec 1, 2015 at 9:38 PM, Vlad Rozov <
>>>>> v.rozov@datatorrent.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> +1 to keep 120 hard stop enforcement and not to rely
on IDE
>>>>>>> formatting
>>>>>>>> to
>>>>>>>>>> find wrapping point. I agree that wrapping string
literal
>> not
>>>>>> always
>>>>>>>> help
>>>>>>>>>> with readability, but overall I think that enforcing
a hard
>>>> stop
>>>>> on
>>>>>>> the
>>>>>>>>>> line length help with writing better code especially
after
>>>> going
>>>>>>>> through
>>>>>>>>> an
>>>>>>>>>> exercise of fixing all code style violations in the
buffer
>>>>> server.
>>>>>>>>>> Additionally some string literals may span multiple
lines
>> and
>>>>> will
>>>>>>>>> require
>>>>>>>>>> breaking anyway.
>>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>>
>>>>>>>>>> Vlad
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12/1/15 21:19, Thomas Weise wrote:
>>>>>>>>>>
>>>>>>>>>>> A while ago, we discussed max length for line
length
>>>> enforcement
>>>>>> and
>>>>>>>>>>> majority wanted to stop at 120 characters.
>>>>>>>>>>>
>>>>>>>>>>> Since then Vlad has fixed code style violations
for one of
>>> the
>>>>>>>> modules:
>>>>>>>>>>> https://github.com/apache/incubator-apex-core/pull/175
>>>>>>>>>>>
>>>>>>>>>>> Before we continue I would like to put the line
length
>>>>> enforcement
>>>>>>>> back
>>>>>>>>>>> for
>>>>>>>>>>> poll.
>>>>>>>>>>>
>>>>>>>>>>> I think it leads to undesirable results, such
as breaking
>>>> string
>>>>>>>>> literals.
>>>>>>>>>>> There are also instances of questionable readability
gains
>>> and
>>>>> the
>>>>>>>>> breaks
>>>>>>>>>>> still have to be manually handled due to unwelcome
IDE
>>>>>> auto-format.
>>>>>>>>>>> My preference would be not not enforce a line
length.
>>>>>>>>>>>
>>>>>>>>>>> Opinions please.
>>>>>>>>>>>
>>>>>>>>>>>


Mime
View raw message