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: [CodeStyle] Maximum length of a line
Date Fri, 30 Oct 2015 01:43:15 GMT
I think we need to go with a reasonable setting for community as not 
everyone may have high resolution or wide monitors and will start 
wrapping code based on their monitor width and not the maximum permitted 
length. As we can only enforce maximum length, but can't enforce 
wrapping, we will end up with code that wraps inconsistently.

Another point not to go with very long line length is github/IDEs side 
by side diff.

Thank you,

Vlad

On 10/29/15 17:52, Pramod Immaneni wrote:
> +1 for 120
>
> On Thu, Oct 29, 2015 at 5:45 PM, Sasha Parfenov <sasha@datatorrent.com>
> wrote:
>
>> +1 for 160 or unlimited.
>>
>> On Thu, Oct 29, 2015 at 3:58 PM, David Yan <david@datatorrent.com> wrote:
>>
>>> I'm +1 on 120 since I'm using a tall screen to code now. :)
>>>
>>> Did we discuss how the code should be indented for lines that break
>> because
>>> of the limit?
>>> For example,
>>>
>>> public void test(int a,
>>>      int b,
>>>      int c)
>>>
>>> or
>>>
>>> public void test(int a,
>>>                   int b,
>>>                   int c)
>>>
>>> The first one uses a fixed width of 4 extra spaces to indent the
>>> continuations, and you won't need to add spaces to the continuation lines
>>> if you rename "test" to be "foo" or "somelongfunctionname".
>>>
>>> But the second one looks better to the eye.
>>>
>>> Also, should the operator be at the end of the line or at the beginning
>> of
>>> the line?
>>>
>>> For example:
>>>
>>> if (a ||
>>>      b &&
>>>      c)
>>>
>>> or
>>>
>>> if (a
>>>    || b
>>>    && c)
>>>
>>> David
>>>
>>> On Thu, Oct 29, 2015 at 3:57 PM, Siyuan Hua <siyuan@datatorrent.com>
>>> wrote:
>>>
>>>> +1 for 160 for 4k monitors
>>>>
>>>> On Thu, Oct 29, 2015 at 3:55 PM, Timothy Farkas <tim@datatorrent.com>
>>>> wrote:
>>>>
>>>>> +1 for 120
>>>>>
>>>>> On Thu, Oct 29, 2015 at 2:28 PM, Chandni Singh <
>>> chandni@datatorrent.com>
>>>>> wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> As part of the discussion below it was brought out that we need to
>>>>> improve
>>>>>> the readability of our code and therefore impose a maximum line
>>> limit.
>>>>>> If you have a preference for maximum length of a line in the code,
>>> then
>>>>>> please voice it now.
>>>>>>
>>>>>> So far Brennon, Ram, Vlad prefer 120.
>>>>>> I am with Thomas on 160.
>>>>>>
>>>>>> This rule can be enforced by checkstyle. However I have also
>> noticed
>>>>>> prematurely wrapping lines, for example,
>>>>>>
>>>>>> public void tes(int t,
>>>>>>                           int x,
>>>>>>                            int z);
>>>>>>
>>>>>> This shouldn't be allowed as well. However checkstyle cannot
>> enforce
>>>>> this.
>>>>>> So it's the responsibility of developers and reviewers to correct
>>> this.
>>>>>> Thanks,
>>>>>> Chandni
>>>>>>
>>>>>> On Thu, Oct 29, 2015 at 10:55 AM, Chandni Singh <
>>>> chandni@datatorrent.com
>>>>>> wrote:
>>>>>>
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Do we have a consensus on line length?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Chandni
>>>>>>>
>>>>>>> On Tue, Oct 20, 2015 at 4:38 PM, Vlad Rozov <
>>> v.rozov@datatorrent.com
>>>>>>> wrote:
>>>>>>>
>>>>>>>> It depends, some screen are tall, not wide :-) . I tried
160 on
>> my
>>>>> tall
>>>>>>>> screen and it looks OK, but I use smaller font size. My
>>>> recommendation
>>>>>> is
>>>>>>>> to keep it to 120.
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>>
>>>>>>>> Vlad
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/20/15 15:38, Thomas Weise wrote:
>>>>>>>>
>>>>>>>>> How about being a bit more generous with line length:
160?
>>> Screens
>>>>> are
>>>>>>>>> pretty wide nowadays..
>>>>>>>>>
>>>>>>>>> +1 for documentation as part of PR review. Documentation
should
>>> not
>>>>>> only
>>>>>>>>> be
>>>>>>>>> present but also helpful. Maybe we can enforce presence
for
>> newly
>>>>> added
>>>>>>>>> classes by tracking count of existing violations?
>>>>>>>>>
>>>>>>>>> On Tue, Oct 20, 2015 at 2:38 PM, Munagala Ramanath <
>>>>>> ram@datatorrent.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Yes, 120 line length max is good.
>>>>>>>>>> Yes, agree we need to find some way to enforce javadocs.
>>>>>>>>>>
>>>>>>>>>> Ram
>>>>>>>>>>
>>>>>>>>>> On Tue, Oct 20, 2015 at 11:45 AM, York, Brennon <
>>>>>>>>>> Brennon.York@capitalone.com
>>>>>>>>>>
>>>>>>>>>>> wrote:
>>>>>>>>>>> For 1 and 2 I¹ve made a JIRA to track
>>>>>>>>>>> (https://malhar.atlassian.net/browse/APEX-204).
>>>>>>>>>>>
>>>>>>>>>>> For 3) I definitely agree we need line wraps.
Understand that
>>>>>> everyone
>>>>>>>>>> has
>>>>>>>>>>
>>>>>>>>>>> different length monitors, but, as a community,
we should
>> agree
>>>> on
>>>>> a
>>>>>>>>>>> standard moving forward as this becomes a community-owned
>>>> project.
>>>>>> How
>>>>>>>>>>> does 120 sound?
>>>>>>>>>>>
>>>>>>>>>>> For 4) if we want to start treating Apex as an
Apache project
>>>> owned
>>>>>> by
>>>>>>>>>> the
>>>>>>>>>>
>>>>>>>>>>> community that uses it we need to start working
*for* the
>>>>> community /
>>>>>>>>>>> developers who are going to contribute to it,
not merely
>>> continue
>>>>> on
>>>>>> as
>>>>>>>>>> if
>>>>>>>>>>
>>>>>>>>>>> the people currently working on it will be the
only primary
>>>>> drivers.
>>>>>>>>>>> That
>>>>>>>>>>> won¹t engender growth or community engagement.
If nothing
>> else
>>> we
>>>>>>>>>>> should
>>>>>>>>>>> be prepared to open our doors to new ideas and
functionality
>> to
>>>> the
>>>>>>>>>>> project, not make it more difficult through obfuscated
code.
>> It
>>>>>> hasn¹t
>>>>>>>>>>> been done to this point and that¹s fine, but
moving forward I
>>>> think
>>>>>> we
>>>>>>>>>>> should take a concerted look and take this as
an opportunity
>> to
>>>>> clean
>>>>>>>>>>> it
>>>>>>>>>>> up / document it. It will only get harder as
the project
>> gains
>>>>>>>>>>> momentum.
>>>>>>>>>>> And, if this causes failures, that¹s a problem
for us to
>> admit,
>>>>>> accept,
>>>>>>>>>>> and fix.
>>>>>>>>>>>
>>>>>>>>>>> On 10/20/15, 10:19 AM, "Chandni Singh" <
>>> chandni@datatorrent.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> 1) This is a bug and will fix this
>>>>>>>>>>>> 2) Another bug and will fix this
>>>>>>>>>>>>
>>>>>>>>>>>> 3) We don't have a line limit because everyone
uses
>> different
>>>>> length
>>>>>>>>>>>> monitor and some prefer a much longer line.
However I think
>> we
>>>>> need
>>>>>> to
>>>>>>>>>>> at
>>>>>>>>>>> least have a minimum length limit and only beyond
this a line
>>>>> should
>>>>>> be
>>>>>>>>>>>> wrapped.
>>>>>>>>>>>>
>>>>>>>>>>>> 4) Earlier javadocs were strictly added to
api and common
>>>> classes.
>>>>>>>>>>>> There
>>>>>>>>>>>> are hardly any for engine, bufferserver modules.
Adding this
>>>> will
>>>>>> mean
>>>>>>>>>>>> much higher number of pre-existing failures.
I am not much
>> in
>>>>> favor
>>>>>> of
>>>>>>>>>>>> this.
>>>>>>>>>>>>
>>>>>>>>>>>> As far as the lineage is concerned, these
were mostly taken
>>> from
>>>>>>>>>>>> google-checks and modified for the style
we adopted. Also
>>>> referred
>>>>>> to
>>>>>>>>>>>> sun_checks and picked a few from there which
we needed.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Oct 20, 2015 at 8:42 AM, Ganelin,
Ilya
>>>>>>>>>>>> <Ilya.Ganelin@capitalone.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> All - there are some issues I¹ve already
run into with the
>>>>>>>>>>>>> CodeStyle/CheckStyle settings. I suggest
we start a JIRA to
>>>> track
>>>>>>>>>>>> these
>>>>>>>>>>> unless you have a preferred approach.
>>>>>>>>>>>>> 1) CheckStyle dictates that chained method
calls be on
>>>> different
>>>>>>>>>>>>> lines
>>>>>>>>>>>>> but
>>>>>>>>>>>>> also dictates that a space may not precede
a period. The
>>> below
>>>> is
>>>>>>>>>>>>> thus
>>>>>>>>>>>>> invalid:
>>>>>>>>>>>>>       Foo.bar
>>>>>>>>>>>>>            .cat
>>>>>>>>>>>>> 2) Continuation Indent is set to 4 in
CheckStyle but set
>> to 2
>>>> by
>>>>>>>>>>>> default
>>>>>>>>>>> in CodeStyle
>>>>>>>>>>>>> 3) We should really enforce line limits
(for the sake of
>>>>>> readability)
>>>>>>>>>>>>> and
>>>>>>>>>>>>> should therefore amend the wrapping behavior
of methods.
>>>> However,
>>>>>>>>>>>>> this
>>>>>>>>>>>>> will
>>>>>>>>>>>>> require updating CheckStyle as well.
>>>>>>>>>>>>> 4) We should enforce JavaDocs
>>>>>>>>>>>>>
>>>>>>>>>>>>> As an aside, could someone possibly speak
to the lineage of
>>> the
>>>>>>>>>>>>> CheckStyle
>>>>>>>>>>>>> and CodeStyle settings that we¹re presently
using inside
>>> Apex?
>>>>> Did
>>>>>>>>>>>> these
>>>>>>>>>>> come from published settings (e.g. Google) or
are these all
>>>>> in-house?
>>>>>>>>>>>>> Appreciate any input, thanks!
>>>>>>>>>>>>> ________________________________________________________
>>>>>>>>>>>>>
>>>>>>>>>>>>> The information contained in this e-mail
is confidential
>>> and/or
>>>>>>>>>>>>> proprietary to Capital One and/or its
affiliates and may
>> only
>>>> be
>>>>>> used
>>>>>>>>>>>>> solely in performance of work or services
for Capital One.
>>> The
>>>>>>>>>>>>> information
>>>>>>>>>>>>> transmitted herewith is intended only
for use by the
>>> individual
>>>>> or
>>>>>>>>>>>>> entity
>>>>>>>>>>>>> to which it is addressed. If the reader
of this message is
>>> not
>>>>> the
>>>>>>>>>>>>> intended
>>>>>>>>>>>>> recipient, you are hereby notified that
any review,
>>>>> retransmission,
>>>>>>>>>>>>> dissemination, distribution, copying
or other use of, or
>>> taking
>>>>> of
>>>>>>>>>>>>> any
>>>>>>>>>>>>> action in reliance upon this information
is strictly
>>>> prohibited.
>>>>> If
>>>>>>>>>>>> you
>>>>>>>>>>> have received this communication in error, please
contact the
>>>>> sender
>>>>>>>>>>>> and
>>>>>>>>>>> delete the material from your computer.
>>>>>>>>>>>>> ________________________________________________________
>>>>>>>>>>>>>
>>>>>>>>>>>>> The information contained in this e-mail
is confidential
>>> and/or
>>>>>>>>>>>>> proprietary to Capital One and/or its
affiliates and may
>> only
>>>> be
>>>>>> used
>>>>>>>>>>>>> solely in performance of work or services
for Capital One.
>>> The
>>>>>>>>>>>>> information
>>>>>>>>>>>>> transmitted herewith is intended only
for use by the
>>> individual
>>>>> or
>>>>>>>>>>>>> entity
>>>>>>>>>>>>> to which it is addressed. If the reader
of this message is
>>> not
>>>>> the
>>>>>>>>>>>>> intended
>>>>>>>>>>>>> recipient, you are hereby notified that
any review,
>>>>> retransmission,
>>>>>>>>>>>>> dissemination, distribution, copying
or other use of, or
>>> taking
>>>>> of
>>>>>>>>>>>>> any
>>>>>>>>>>>>> action in reliance upon this information
is strictly
>>>> prohibited.
>>>>> If
>>>>>>>>>>>> you
>>>>>>>>>>> have received this communication in error, please
contact the
>>>>> sender
>>>>>>>>>>>> and
>>>>>>>>>>> delete the material from your computer.
>>>>>>>>>>>>>
>>>>>>>>>>>>> ________________________________________________________
>>>>>>>>>>> The information contained in this e-mail is confidential
>> and/or
>>>>>>>>>>> proprietary to Capital One and/or its affiliates
and may only
>>> be
>>>>> used
>>>>>>>>>>> solely in performance of work or services for
Capital One.
>> The
>>>>>>>>>> information
>>>>>>>>>>
>>>>>>>>>>> transmitted herewith is intended only for use
by the
>> individual
>>>> or
>>>>>>>>>>> entity
>>>>>>>>>>> to which it is addressed. If the reader of this
message is
>> not
>>>> the
>>>>>>>>>> intended
>>>>>>>>>>
>>>>>>>>>>> recipient, you are hereby notified that any review,
>>>> retransmission,
>>>>>>>>>>> dissemination, distribution, copying or other
use of, or
>> taking
>>>> of
>>>>>> any
>>>>>>>>>>> action in reliance upon this information is strictly
>>> prohibited.
>>>> If
>>>>>> you
>>>>>>>>>>> have received this communication in error, please
contact the
>>>>> sender
>>>>>>>>>>> and
>>>>>>>>>>> delete the material from your computer.
>>>>>>>>>>>
>>>>>>>>>>> ________________________________________________________
>>>>>>>>>>>
>>>>>>>>>>> The information contained in this e-mail is confidential
>> and/or
>>>>>>>>>>> proprietary to Capital One and/or its affiliates
and may only
>>> be
>>>>> used
>>>>>>>>>>> solely in performance of work or services for
Capital One.
>> The
>>>>>>>>>> information
>>>>>>>>>>
>>>>>>>>>>> transmitted herewith is intended only for use
by the
>> individual
>>>> or
>>>>>>>>>>> entity
>>>>>>>>>>> to which it is addressed. If the reader of this
message is
>> not
>>>> the
>>>>>>>>>> intended
>>>>>>>>>>
>>>>>>>>>>> recipient, you are hereby notified that any review,
>>>> retransmission,
>>>>>>>>>>> dissemination, distribution, copying or other
use of, or
>> taking
>>>> of
>>>>>> any
>>>>>>>>>>> action in reliance upon this information is strictly
>>> prohibited.
>>>> If
>>>>>> you
>>>>>>>>>>> have received this communication in error, please
contact the
>>>>> sender
>>>>>>>>>>> and
>>>>>>>>>>> delete the material from your computer.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>


Mime
View raw message