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 and CheckStyle Inconsistencies
Date Thu, 29 Oct 2015 17:55:09 GMT
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