apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chandni Singh <chan...@datatorrent.com>
Subject [CodeStyle] Maximum length of a line
Date Thu, 29 Oct 2015 22:28:15 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message