One more question, what do you differ
*public **void func(*
* int arg1,*
* int arg2,*
* ...)** throws E1, E2, E3 {*
* ...*
*}*
and
*public **void func(*
* int arg1,*
* int arg2,*
* ...
*)** throws E1, E2, E3 {*
* ...*
*}*
I prefer the latter because parentheses are aligned in a similar way,
as well as the border between declaration and function body is clear.
Zili Chen <wander4...@gmail.com> 于2019年8月22日周四 上午9:53写道：
> Thanks Andrey for driving the discussion. Just for clarification,
> what we conclude here are several guidelines without automatic
> checker/tool guard them, right?
>
> Best,
> tison.
>
>
> Andrey Zagrebin <and...@ververica.com> 于2019年8月21日周三 下午8:18写道：
>
>> Hi All,
>>
>> I suggest we also conclude this discussion now.
>>
>> Breaking the line of too long statements (line longness is yet to be fully
>> defined) to improve code readability in case of
>>
>> - Long function argument lists (declaration or call): void func(type1
>> arg1, type2 arg2, ...)
>> - Long sequence of chained calls:
>> list.stream().map(...).reduce(...).collect(...)...
>>
>> Rules:
>>
>> - Break the list of arguments/calls if the line exceeds limit or
>> earlier
>> if you believe that the breaking would improve the code readability
>> - If you break the line then each argument/call should have a separate
>> line, including the first one
>> - Each new line argument/call should have one extra indentation
>> relative
>> to the line of the parent function name or called entity
>> - The opening brace always stays on the line of the parent function
>> name
>> - The closing brace of the function argument list and the possible
>> thrown exception list always stay on the line of the last argument
>> - The dot of a chained call is always on the line of that chained call
>> proceeding the call at the beginning
>>
>> Examples of breaking:
>>
>> - Function arguments
>>
>> *public **void func(*
>> * int arg1,*
>> * int arg2,*
>> * ...)** throws E1, E2, E3 {*
>> * ...*
>> *}*
>>
>>
>> - Chained method calls:
>>
>> *values*
>> * .stream()*
>> * .map(*...*)*
>> * .collect(...);*
>>
>>
>> I suggest we spawn separate discussion threads (can do as a follow-up)
>> about:
>>
>> - the hard line length limit in Java, possibly to confirm it also for
>> Scala (cc @Tison)
>> - indentation rules for the broken list of a declared function
>> arguments
>>
>> If there are no more comments/objections/concerns, I will open a PR to
>> capture the discussion outcome.
>>
>> Best,
>> Andrey
>>
>>
>>
>> On Wed, Aug 21, 2019 at 8:57 AM Zili Chen <wander4...@gmail.com> wrote:
>>
>> > Implement question: how to apply the line length rules?
>> >
>> > If we just turn on checkstyle rule "LineLength" then a huge
>> > effort is required to break lines those break the rule. If
>> > we use an auto-formatter here then it possibly break line
>> > "just at the position" awfully.
>> >
>> > Is it possible we require only to fit the rule on the fly
>> > a pull request touch files?
>> >
>> > Best,
>> > tison.
>> >
>> >
>> > Yu Li <car...@gmail.com> 于2019年8月20日周二 下午5:22写道：
>> >
>> > > I second Stephan's summarize, and to be more explicit, +1 on:
>> > > - Set a hard line length limit
>> > > - Allow arguments on the same line if below length limit
>> > > - With consistent argument breaking when that length is exceeded
>> > > - Developers can break before that if they feel it helps with
>> readability
>> > >
>> > > FWIW, hbase project also sets the line length limit to 100 [1]
>> > (personally
>> > > I don't have any tendency on this, so JFYI).
>> > >
>> > > [1]
>> > >
>> > >
>> >
>> https://github.com/apache/hbase/blob/a59f7d4ffc27ea23b9822c3c26d6aeb76ccdf9aa/hbase-checkstyle/src/main/resources/hbase/checkstyle.xml#L128
>> > >
>> > > Best Regards,
>> > > Yu
>> > >
>> > >
>> > > On Mon, 19 Aug 2019 at 18:22, Stephan Ewen <se...@apache.org> wrote:
>> > >
>> > > > I personally prefer not to break lines with few parameters.
>> > > > It just feels unnecessarily clumsy to parse the breaks if there are
>> > only
>> > > > two or three arguments with short names.
>> > > >
>> > > > So +1
>> > > > - for a hard line length limit
>> > > > - allowing arguments on the same line if below that limit
>> > > > - with consistent argument breaking when that length is exceeded
>> > > > - developers can break before that if they feel it helps with
>> > > > readability.
>> > > >
>> > > > This should be similar to what we have, except for enforcing the
>> line
>> > > > length limit.
>> > > >
>> > > > I think our Java guide originally suggested 120 characters line
>> length,
>> > > but
>> > > > we can reduce that to 100 if a majority argues for that, but it is
>> > > separate
>> > > > discussion.
>> > > > We uses shorter lines in Scala (100 chars) because Scala code
>> becomes
>> > > > harder to read faster with long lines.
>> > > >
>> > > >
>> > > > On Mon, Aug 19, 2019 at 10:45 AM Andrey Zagrebin <
>> and...@ververica.com
>> > >
>> > > > wrote:
>> > > >
>> > > > > Hi Everybody,
>> > > > >
>> > > > > Thanks for your feedback guys and sorry for not getting back to
>> the
>> > > > > discussion for some time.
>> > > > >
>> > > > > @SHI Xiaogang
>> > > > > About breaking lines for thrown exceptions:
>> > > > > Indeed that would prevent growing the throw clause indefinitely.
>> > > > > I am a bit concerned about putting the right parenthesis and/or
>> throw
>> > > > > clause on the next line
>> > > > > because in general we do not it and there are a lot of variations
>> of
>> > > how
>> > > > > and what to put to the next line so it needs explicit memorising.
>> > > > > Also, we do not have many checked exceptions and usually avoid
>> them.
>> > > > > Although I am not a big fan of many function arguments either but
>> > this
>> > > > > seems to be a bigger problem in the code base.
>> > > > > I would be ok to not enforce anything for exceptions atm.
>> > > > >
>> > > > > @Chesnay Schepler <ches...@apache.org>
>> > > > > Thanks for mentioning automatic checks.
>> > > > > Indeed, pointing out this kind of style issues during PR reviews
>> is
>> > > very
>> > > > > tedious
>> > > > > and cannot really force it without automated tools.
>> > > > > I would still consider the outcome of this discussion as a soft
>> > > > > recommendation atm (which we also have for some other things in
>> the
>> > > code
>> > > > > style draft).
>> > > > > We need more investigation about how to enforce things. I am not
>> so
>> > > > > knowledgable about code style/IDE checks.
>> > > > > From the first glance I also do not see a simple way. If somebody
>> has
>> > > > more
>> > > > > insight please share your experience.
>> > > > >
>> > > > > @Biao Liu <mmyy1...@gmail.com>
>> > > > > Line length limitation:
>> > > > > I do not see anything for Java, only for Scala: 100 (also
>> enforced by
>> > > > build
>> > > > > AFAIK).
>> > > > > From what I heard there has been already some discussion about the
>> > hard
>> > > > > limit for the line length.
>> > > > > Although quite some people are in favour of it (including me) and
>> > seems
>> > > > to
>> > > > > be a nice limitation,
>> > > > > there are some practical implication about it.
>> > > > > Historically, Flink did not have any code style checks and huge
>> > chunks
>> > > of
>> > > > > code base have to be reformatted destroying the commit history.
>> > > > > Another thing is value for the limit. Nowadays, we have wide
>> screens
>> > > and
>> > > > do
>> > > > > not often even need to scroll.
>> > > > > Nevertheless, we can kick off another discussion about the line
>> > length
>> > > > > limit and enforcing it.
>> > > > > Atm I see people to adhere to a soft recommendation of 120 line
>> > length
>> > > > for
>> > > > > Java because it is usually a bit more verbose comparing to Scala.
>> > > > >
>> > > > > *Question 1*:
>> > > > > I would be ok to always break line if there is more than one
>> chained
>> > > > call.
>> > > > > There are a lot of places where this is only one short call I
>> would
>> > not
>> > > > > break line in this case.
>> > > > > If it is too confusing I would be ok to stick to the rule to break
>> > > either
>> > > > > all or none.
>> > > > > Thanks for pointing out this explicitly: For a chained method
>> calls,
>> > > the
>> > > > > new line should be started with the dot.
>> > > > > I think it should be also part of the rule if forced.
>> > > > >
>> > > > > *Question 2:*
>> > > > > The indent of new line should be 1 tab or 2 tabs? (I assume it
>> > matters
>> > > > only
>> > > > > for function arguments)
>> > > > > This is a good point which again probably deserves a separate
>> thread.
>> > > > > We also had an internal discussion about it. I would be also in
>> > favour
>> > > of
>> > > > > resolving it into one way.
>> > > > > Atm we indeed have 2 ways in our code base which are again soft
>> > > > > recommendations.
>> > > > > The problem is mostly with enforcing it automatically.
>> > > > > The approach with extra indentation also needs IDE setup
>> otherwise it
>> > > is
>> > > > > annoying
>> > > > > that after every function cut/paste, e.g. Idea changes the format
>> to
>> > > one
>> > > > > indentation automatically and often people do not notice it.
>> > > > >
>> > > > > I suggest we still finish this discussion to a point of achieving
>> a
>> > > soft
>> > > > > recommendation which we can also reconsider
>> > > > > when there are more ideas about automatically enforcing these
>> things.
>> > > > >
>> > > > > Best,
>> > > > > Andrey
>> > > > >
>> > > > > On Sat, Aug 3, 2019 at 7:51 AM SHI Xiaogang <
>> shixiaoga...@gmail.com>
>> > > > > wrote:
>> > > > >
>> > > > > > Hi Chesnay,
>> > > > > >
>> > > > > > Thanks a lot for your reminder.
>> > > > > >
>> > > > > > For Intellij settings, the style i proposed can be configured as
>> > > below
>> > > > > > * Method declaration parameters: chop down if long
>> > > > > > * align when multiple: YES
>> > > > > > * new line after '(': YES
>> > > > > > * place ')' on new line: YES
>> > > > > > * Method call arguments: chop down if long
>> > > > > > * align when multiple: YES
>> > > > > > * take priority over call chain wrapping: YES
>> > > > > > * new line after '(': YES
>> > > > > > * place ')' on new line: YES
>> > > > > > * Throws list: chop down if long
>> > > > > > * align when multiline: YES
>> > > > > >
>> > > > > > As far as i know, there does not exist standard checks for the
>> > > > alignment
>> > > > > of
>> > > > > > method parameters or arguments. It needs further investigation
>> to
>> > see
>> > > > > > whether we can validate these styles via customized checks.
>> > > > > >
>> > > > > >
>> > > > > > Biao Liu <mmyy1...@gmail.com> 于2019年8月2日周五 下午4:00写道：
>> > > > > >
>> > > > > > > Hi Andrey,
>> > > > > > >
>> > > > > > > Thank you for bringing us this discussion.
>> > > > > > >
>> > > > > > > I would like to make some details clear. Correct me if I am
>> > wrong.
>> > > > > > >
>> > > > > > > The guide draft [1] says the line length is limited in 100
>> > > > characters.
>> > > > > > From
>> > > > > > > my understanding, this discussion suggests if there is more
>> than
>> > > 100
>> > > > > > > characters in one line (both Scala and Java), we should start
>> a
>> > new
>> > > > > line
>> > > > > > > (or lines).
>> > > > > > >
>> > > > > > > *Question 1*: If a line does not exceed 100 characters,
>> should we
>> > > > break
>> > > > > > the
>> > > > > > > chained calls into lines? Currently the chained calls always
>> been
>> > > > > broken
>> > > > > > > into lines even it's not too long. Does it just a suggestion
>> or a
>> > > > > > > limitation?
>> > > > > > > I prefer it's a limitation which must be respected. And we
>> should
>> > > > > always
>> > > > > > > break the chained calls no matter how long the line is.
>> > > > > > >
>> > > > > > > For a chained method calls, the new line should be started
>> with
>> > the
>> > > > > dot.
>> > > > > > >
>> > > > > > > *Question 2:* The indent of new line should be 1 tab or 2
>> tabs?
>> > > > > Currently
>> > > > > > > there exists these two different styles. This rule should be
>> also
>> > > > > applied
>> > > > > > > to function arguments.
>> > > > > > >
>> > > > > > > BTW, big +1 to options from Chesnay. We should make
>> auto-format
>> > > > > possible
>> > > > > > on
>> > > > > > > our project.
>> > > > > > >
>> > > > > > > 1.
>> > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ/edit#
>> > > > > > >
>> > > > > > > Thanks,
>> > > > > > > Biao /'bɪ.aʊ/
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > > > > On Fri, Aug 2, 2019 at 9:20 AM SHI Xiaogang <
>> > > shixiaoga...@gmail.com>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Hi Andrey,
>> > > > > > > >
>> > > > > > > > Thanks for bringing this. Personally, I prefer to the
>> following
>> > > > style
>> > > > > > > which
>> > > > > > > > (1) puts the right parenthese in the next line
>> > > > > > > > (2) a new line for each exception if exceptions can not be
>> put
>> > in
>> > > > the
>> > > > > > > same
>> > > > > > > > line
>> > > > > > > >
>> > > > > > > > That way, parentheses are aligned in a similar way to braces
>> > and
>> > > > > > > exceptions
>> > > > > > > > can be well aligned.
>> > > > > > > >
>> > > > > > > > *public **void func(*
>> > > > > > > > * int arg1,*
>> > > > > > > > * int arg2,*
>> > > > > > > > * ...
>> > > > > > > > *) throws E1, E2, E3 {*
>> > > > > > > > * ...
>> > > > > > > > *}*
>> > > > > > > >
>> > > > > > > > or
>> > > > > > > >
>> > > > > > > > *public **void func(*
>> > > > > > > > * int arg1,*
>> > > > > > > > * int arg2,*
>> > > > > > > > * ...
>> > > > > > > > *) throws
>> > > > > > > > * E1,
>> > > > > > > > * E2,
>> > > > > > > > * E3 {*
>> > > > > > > > * ...
>> > > > > > > > *}*
>> > > > > > > >
>> > > > > > > > Regards,
>> > > > > > > > Xiaogang
>> > > > > > > >
>> > > > > > > > Andrey Zagrebin <and...@ververica.com> 于2019年8月1日周四
>> 下午11:19写道：
>> > > > > > > >
>> > > > > > > > > Hi all,
>> > > > > > > > >
>> > > > > > > > > This is one more small suggestion for the recent thread
>> about
>> > > > code
>> > > > > > > style
>> > > > > > > > > guide in Flink [1].
>> > > > > > > > >
>> > > > > > > > > We already have a note about using a new line for each
>> > chained
>> > > > call
>> > > > > > in
>> > > > > > > > > Scala, e.g. either:
>> > > > > > > > >
>> > > > > > > > > *values**.stream()**.map(...)**,collect(...);*
>> > > > > > > > >
>> > > > > > > > > or
>> > > > > > > > >
>> > > > > > > > > *values*
>> > > > > > > > > * .stream()*
>> > > > > > > > > * .map(*...*)*
>> > > > > > > > > * .collect(...)*
>> > > > > > > > >
>> > > > > > > > > if it would result in a too long line by keeping all
>> chained
>> > > > calls
>> > > > > in
>> > > > > > > one
>> > > > > > > > > line.
>> > > > > > > > >
>> > > > > > > > > The suggestion is to have it for Java as well and add the
>> > same
>> > > > rule
>> > > > > > > for a
>> > > > > > > > > long list of function arguments. So it is either:
>> > > > > > > > >
>> > > > > > > > > *public void func(int arg1, int arg2, ...) throws E1, E2,
>> E3
>> > {*
>> > > > > > > > > * ...*
>> > > > > > > > > *}*
>> > > > > > > > >
>> > > > > > > > > or
>> > > > > > > > >
>> > > > > > > > > *public **void func(*
>> > > > > > > > > * int arg1,*
>> > > > > > > > > * int arg2,*
>> > > > > > > > > * ...)** throws E1, E2, E3 {*
>> > > > > > > > > * ...*
>> > > > > > > > > *}*
>> > > > > > > > >
>> > > > > > > > > but thrown exceptions stay on the same last line.
>> > > > > > > > >
>> > > > > > > > > Please, feel free to share you thoughts.
>> > > > > > > > >
>> > > > > > > > > Best,
>> > > > > > > > > Andrey
>> > > > > > > > >
>> > > > > > > > > [1]
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
