[DISCUSS][CODE STYLE] Breaking long function argument lists and chained method calls

[Andrey Zagrebin-3]

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-85BC710FD157@...%3E

[SHI Xiaogang]

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 <[hidden email]> 于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-85BC710FD157@...%3E
>

[Chesnay Schepler-3]

Just so everyone remembers:

Any suggested code-style should be
a) configurable in the IDE (otherwise we'll never be able to auto-format)
b) be verifiable via checkstyle (otherwise we'll end up manually
checking for code-style again)

On 02/08/2019 03:20, SHI Xiaogang 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 <[hidden email]> 于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-85BC710FD157@...%3E
>>

[Biao Liu]

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 <[hidden email]> 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 <[hidden email]> 于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-85BC710FD157@...%3E
> >
>

[SHI Xiaogang]

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 <[hidden email]> 于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 <[hidden email]>
> 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > >
> >
>

[Andrey Zagrebin-3]

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 <[hidden email]>
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 <[hidden email]>
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 <[hidden email]> 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 <[hidden email]> 于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 <[hidden email]>
> > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > >
> > >
> >
>

[Stephan Ewen]

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 <[hidden email]>
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 <[hidden email]>
> 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 <[hidden email]>
> 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 <[hidden email]>
> 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 <[hidden email]> 于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 <[hidden email]>
> > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > > >
> > > >
> > >
> >
>

[Yu Li]

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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
> > 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 <[hidden email]>
> > 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 <[hidden email]>
> > 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 <[hidden email]> 于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 <[hidden email]>
> > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > > > >
> > > > >
> > > >
> > >
> >
>

[tison]

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 <[hidden email]> 于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 <[hidden email]> 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 <[hidden email]>
> > 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 <[hidden email]>
> > > 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 <[hidden email]>
> > > 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 <[hidden email]>
> > > 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 <[hidden email]> 于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 <
> [hidden email]>
> > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

[Andrey Zagrebin-3]

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 <[hidden email]> 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 <[hidden email]> 于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 <[hidden email]> 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 <[hidden email]
> >
> > > 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 <[hidden email]>
> > > > 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 <[hidden email]>
> > > > 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 <[hidden email]>
> > > > 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 <[hidden email]> 于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 <
> > [hidden email]>
> > > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

[tison]

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 <[hidden email]> 于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 <[hidden email]> 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 <[hidden email]> 于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 <[hidden email]> 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 <
> [hidden email]
> > >
> > > > 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 <[hidden email]>
> > > > > 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 <[hidden email]>
> > > > > 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 <
> [hidden email]>
> > > > > 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 <[hidden email]> 于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 <
> > > [hidden email]>
> > > > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

[tison]

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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 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 <[hidden email]> 于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 <[hidden email]> 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 <
>> [hidden email]
>> > >
>> > > > 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 <[hidden email]>
>> > > > > 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 <[hidden email]>
>> > > > > 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 <
>> [hidden email]>
>> > > > > 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 <[hidden email]> 于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 <
>> > > [hidden email]>
>> > > > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>

[Andrey Zagrebin-3]

Hi Tison,

Regarding the automatic checks.
Yes, I suggest we conclude the discussion without the automatic checks.
As soon as we have more ideas/investigation, put into automation, we can
activate it and/or reconsider.
Nonetheless, I do not see any problem if we agree on this atm and make it
part of our code style recommendations.

Regarding putting the right parenthesis on the new line.
At the moment we do not use this approach in our code base. My personal
feeling is that it is not so often used in Java.
Anyways I think, this goes more into direction of the second follow-up to
discuss this separately:

   - indentation rules for the broken list of a declared function arguments
   (atm we usually do: one and newline before body or two idents)

or maybe we should rather name it: how to separate function declaration and
body (as you already mentioned it like this).

We can change this point:

   - The closing brace of the function argument list and the possible
   thrown exception list always stay on the line of the last argument

to

   - The possible thrown exception list is never broken and stays on the
   same last line

Then we can also adjust it if needed after the discussion about how to
separate function declaration and body.

Best,
Andrey






On Thu, Aug 22, 2019 at 9:05 AM Zili Chen <[hidden email]> wrote:

> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]> 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 <[hidden email]> 于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 <[hidden email]>
> 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 <
> >> [hidden email]
> >> > >
> >> > > > 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 <[hidden email]>
> >> > > > > 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 <[hidden email]>
> >> > > > > 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 <
> >> [hidden email]>
> >> > > > > 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 <[hidden email]> 于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 <
> >> > > [hidden email]>
> >> > > > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>

[Andrey Zagrebin-3]

FYI PR: https://github.com/apache/flink-web/pull/254

On Thu, Aug 22, 2019 at 10:11 AM Andrey Zagrebin <[hidden email]>
wrote:

> Hi Tison,
>
> Regarding the automatic checks.
> Yes, I suggest we conclude the discussion without the automatic checks.
> As soon as we have more ideas/investigation, put into automation, we can
> activate it and/or reconsider.
> Nonetheless, I do not see any problem if we agree on this atm and make it
> part of our code style recommendations.
>
> Regarding putting the right parenthesis on the new line.
> At the moment we do not use this approach in our code base. My personal
> feeling is that it is not so often used in Java.
> Anyways I think, this goes more into direction of the second follow-up to
> discuss this separately:
>
>    - indentation rules for the broken list of a declared function
>    arguments (atm we usually do: one and newline before body or two idents)
>
> or maybe we should rather name it: how to separate function declaration
> and body (as you already mentioned it like this).
>
> We can change this point:
>
>    - The closing brace of the function argument list and the possible
>    thrown exception list always stay on the line of the last argument
>
> to
>
>    - The possible thrown exception list is never broken and stays on the
>    same last line
>
> Then we can also adjust it if needed after the discussion about how to
> separate function declaration and body.
>
> Best,
> Andrey
>
>
>
>
>
>
> On Thu, Aug 22, 2019 at 9:05 AM Zili Chen <[hidden email]> wrote:
>
>> 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 <[hidden email]> 于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 <[hidden email]> 于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 <[hidden email]>
>> 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 <[hidden email]> 于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 <[hidden email]>
>> 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 <
>> >> [hidden email]
>> >> > >
>> >> > > > 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 <[hidden email]>
>> >> > > > > 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 <[hidden email]>
>> >> > > > > 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 <
>> >> [hidden email]>
>> >> > > > > 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 <[hidden email]> 于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 <
>> >> > > [hidden email]>
>> >> > > > > > > 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 <[hidden email]> 于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-85BC710FD157@...%3E
>> >> > > > > > > > >
>> >> > > > > > > >
>> >> > > > > > >
>> >> > > > > >
>> >> > > > >
>> >> > > >
>> >> > >
>> >> >
>> >>
>> >
>>
>