Hi Everybody,
Thanks for your feedback guys and sorry for not getting back to the
discussion for some time.
@SHI Xiaogang
About breaking lines for thrown exceptions:
Indeed that would prevent growing the throw clause indefinitely.
I am a bit concerned about putting the right parenthesis and/or throw
clause on the next line
because in general we do not it and there are a lot of variations of how
and what to put to the next line so it needs explicit memorising.
Also, we do not have many checked exceptions and usually avoid them.
Although I am not a big fan of many function arguments either but this
seems to be a bigger problem in the code base.
I would be ok to not enforce anything for exceptions atm.
@Chesnay Schepler <ches...@apache.org>
Thanks for mentioning automatic checks.
Indeed, pointing out this kind of style issues during PR reviews is very
tedious
and cannot really force it without automated tools.
I would still consider the outcome of this discussion as a soft
recommendation atm (which we also have for some other things in the code
style draft).
We need more investigation about how to enforce things. I am not so
knowledgable about code style/IDE checks.
>From the first glance I also do not see a simple way. If somebody has more
insight please share your experience.
@Biao Liu <mmyy1...@gmail.com>
Line length limitation:
I do not see anything for Java, only for Scala: 100 (also enforced by build
AFAIK).
>From what I heard there has been already some discussion about the hard
limit for the line length.
Although quite some people are in favour of it (including me) and seems to
be a nice limitation,
there are some practical implication about it.
Historically, Flink did not have any code style checks and huge chunks of
code base have to be reformatted destroying the commit history.
Another thing is value for the limit. Nowadays, we have wide screens and do
not often even need to scroll.
Nevertheless, we can kick off another discussion about the line length
limit and enforcing it.
Atm I see people to adhere to a soft recommendation of 120 line length for
Java because it is usually a bit more verbose comparing to Scala.
*Question 1*:
I would be ok to always break line if there is more than one chained call.
There are a lot of places where this is only one short call I would not
break line in this case.
If it is too confusing I would be ok to stick to the rule to break either
all or none.
Thanks for pointing out this explicitly: For a chained method calls, the
new line should be started with the dot.
I think it should be also part of the rule if forced.
*Question 2:*
The indent of new line should be 1 tab or 2 tabs? (I assume it matters only
for function arguments)
This is a good point which again probably deserves a separate thread.
We also had an internal discussion about it. I would be also in favour of
resolving it into one way.
Atm we indeed have 2 ways in our code base which are again soft
recommendations.
The problem is mostly with enforcing it automatically.
The approach with extra indentation also needs IDE setup otherwise it is
annoying
that after every function cut/paste, e.g. Idea changes the format to one
indentation automatically and often people do not notice it.
I suggest we still finish this discussion to a point of achieving a soft
recommendation which we can also reconsider
when there are more ideas about automatically enforcing these things.
Best,
Andrey
On Sat, Aug 3, 2019 at 7:51 AM SHI Xiaogang <shixiaoga...@gmail.com> wrote:
> Hi Chesnay,
>
> Thanks a lot for your reminder.
>
> For Intellij settings, the style i proposed can be configured as below
> * Method declaration parameters: chop down if long
> * align when multiple: YES
> * new line after '(': YES
> * place ')' on new line: YES
> * Method call arguments: chop down if long
> * align when multiple: YES
> * take priority over call chain wrapping: YES
> * new line after '(': YES
> * place ')' on new line: YES
> * Throws list: chop down if long
> * align when multiline: YES
>
> As far as i know, there does not exist standard checks for the alignment of
> method parameters or arguments. It needs further investigation to see
> whether we can validate these styles via customized checks.
>
>
> Biao Liu <mmyy1...@gmail.com> 于2019年8月2日周五 下午4:00写道：
>
> > Hi Andrey,
> >
> > Thank you for bringing us this discussion.
> >
> > I would like to make some details clear. Correct me if I am wrong.
> >
> > The guide draft [1] says the line length is limited in 100 characters.
> From
> > my understanding, this discussion suggests if there is more than 100
> > characters in one line (both Scala and Java), we should start a new line
> > (or lines).
> >
> > *Question 1*: If a line does not exceed 100 characters, should we break
> the
> > chained calls into lines? Currently the chained calls always been broken
> > into lines even it's not too long. Does it just a suggestion or a
> > limitation?
> > I prefer it's a limitation which must be respected. And we should always
> > break the chained calls no matter how long the line is.
> >
> > For a chained method calls, the new line should be started with the dot.
> >
> > *Question 2:* The indent of new line should be 1 tab or 2 tabs? Currently
> > there exists these two different styles. This rule should be also applied
> > to function arguments.
> >
> > BTW, big +1 to options from Chesnay. We should make auto-format possible
> on
> > our project.
> >
> > 1.
> >
> >
> https://docs.google.com/document/d/1owKfK1DwXA-w6qnx3R7t2D_o0BsFkkukGlRhvl3XXjQ/edit#
> >
> > Thanks,
> > Biao /'bɪ.aʊ/
> >
> >
> >
> > On Fri, Aug 2, 2019 at 9:20 AM SHI Xiaogang <shixiaoga...@gmail.com>
> > wrote:
> >
> > > Hi Andrey,
> > >
> > > Thanks for bringing this. Personally, I prefer to the following style
> > which
> > > (1) puts the right parenthese in the next line
> > > (2) a new line for each exception if exceptions can not be put in the
> > same
> > > line
> > >
> > > That way, parentheses are aligned in a similar way to braces and
> > exceptions
> > > can be well aligned.
> > >
> > > *public **void func(*
> > > * int arg1,*
> > > * int arg2,*
> > > * ...
> > > *) throws E1, E2, E3 {*
> > > * ...
> > > *}*
> > >
> > > or
> > >
> > > *public **void func(*
> > > * int arg1,*
> > > * int arg2,*
> > > * ...
> > > *) throws
> > > * E1,
> > > * E2,
> > > * E3 {*
> > > * ...
> > > *}*
> > >
> > > Regards,
> > > Xiaogang
> > >
> > > Andrey Zagrebin <and...@ververica.com> 于2019年8月1日周四 下午11:19写道：
> > >
> > > > Hi all,
> > > >
> > > > This is one more small suggestion for the recent thread about code
> > style
> > > > guide in Flink [1].
> > > >
> > > > We already have a note about using a new line for each chained call
> in
> > > > Scala, e.g. either:
> > > >
> > > > *values**.stream()**.map(...)**,collect(...);*
> > > >
> > > > or
> > > >
> > > > *values*
> > > > * .stream()*
> > > > * .map(*...*)*
> > > > * .collect(...)*
> > > >
> > > > if it would result in a too long line by keeping all chained calls in
> > one
> > > > line.
> > > >
> > > > The suggestion is to have it for Java as well and add the same rule
> > for a
> > > > long list of function arguments. So it is either:
> > > >
> > > > *public void func(int arg1, int arg2, ...) throws E1, E2, E3 {*
> > > > * ...*
> > > > *}*
> > > >
> > > > or
> > > >
> > > > *public **void func(*
> > > > * int arg1,*
> > > > * int arg2,*
> > > > * ...)** throws E1, E2, E3 {*
> > > > * ...*
> > > > *}*
> > > >
> > > > but thrown exceptions stay on the same last line.
> > > >
> > > > Please, feel free to share you thoughts.
> > > >
> > > > Best,
> > > > Andrey
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E
> > > >
> > >
> >
>
