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