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