apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Weise <tho...@datatorrent.com>
Subject Re: Code style: Line length enforcement
Date Tue, 22 Dec 2015 22:37:51 GMT
The question was line length enforcement yes/no. Based on the result,
please remove enforcement.

Thomas


On Mon, Dec 21, 2015 at 3:53 PM, Chandni Singh <chandni@datatorrent.com>
wrote:

> Was any decision made here.
>
> From this email thread
> 9 votes for recommendation
> 5 votes for enforcement.
>
> Since more people are in favor of of having this as recommendation, shall I
> go ahead make this change?
>
> Thanks,
> Chandni
>
> On Wed, Dec 2, 2015 at 9:56 AM, York, Brennon <Brennon.York@capitalone.com
> >
> wrote:
>
> > I view this in the same regards as a white line on where to stop at a
> stop
> > light. The federal govt. must say, in clear terms, ³this is where you
> stop
> > at a stop light, NO FARTHER² even though people go past the white line
> all
> > the time. The point is that, if necessary, it can be enforced with
> > *exacting* guidelines. There is *no debate* on whether you can be after
> > the line or not when stopped at a stop light, technically, according to
> > the law.
> >
> > The point is we should set a clear and undisputed line. A recommendation
> > will only cause confusion and debate. A clear stance, one way or the
> > other, is, in my view, the only right way to move forward, even with a
> > decision as seemingly minimal as code line length.
> >
> > All that said I¹m +1 for keeping 120 characters for all the below reasons
> > listed thus far.
> >
> > On 12/2/15, 8:53 AM, "Vlad Rozov" <v.rozov@datatorrent.com> wrote:
> >
> > >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.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >
> >
> > ________________________________________________________
> >
> > 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