apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chandni Singh <chan...@datatorrent.com>
Subject Re: [CodeStyle] Maximum length of a line
Date Fri, 30 Oct 2015 01:45:31 GMT
Vlad,

120 is already leading :-)

Chandni

On Thu, Oct 29, 2015 at 6:43 PM, Vlad Rozov <v.rozov@datatorrent.com> wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message