Re: [DISCUSS] Codestyle: force multiline indentation
Agree vino yang, on your views on relying on check style and spotless mainly. What I meant to say was we at least need a way to ensure IDE formatting don’t get in the way (either via tooling or via docs for eclipse/IntelliJ) I would like to argue that this be a mandatory requirement. IDE can pick up some of the check style rules (that’s how I am living now). The specific issue last time was that checkstyle rules on import order was not picked up. As you move code around the amount of time manually spent fixing import order was not b simply worth the benefits the standardization brings. On Sat, Aug 22, 2020 at 1:48 PM Shiyan Xu wrote: > It can be up to the individual to use the IDE formatter or not, as long as > > there is a tool to help enforce Checkstyle rules. > > For people who use IDE formatter, importing Checkstyle.xml as a format > > scheme does not fully control the formatter's behavior, that's why IDE > > sometimes gets in the way. But most of the time it serves us well. > > Guess we can close the thread as we're all in favor of spotless? > > > > > > > > On Sat, Aug 22, 2020 at 6:08 AM vino yang wrote: > > > > > Hi vc, > > > > > > Yes, this part of the practice may have different preferences for > different > > > developers. I have never opened the IDE's automatic formatting, nor have > I > > > used the IDE's formatting functions artificially. Because I have > > > participated in multiple open source communities, each open source > > > community has its own conventions on code style. So, I just understand > the > > > style of each community, after changing the code, and then compiling > > > locally, checkstyle will identify the related problems, and then report, > > > and then I will modify until the compilation is passed. > > > > > > I admit that this is my personal behavior, and everything has its two > > > sides. IDE automatic formatting will make it more convenient for > developers > > > to deal with code styles. On the other hand, it will also make the > > > community more complicated when considering related conventions and weigh > > > more factors. > > > > > > Best, > > > Vino > > > > > > Vinoth Chandar 于2020年8月22日周六 下午2:25写道： > > > > > > > >But, IMO, we can ignore the IDE here, if it breaks the code style, > > > > checkstyle will stop building and spotless will work. > > > > > > > > I differ here slightly. Most people reformat code using the "format > code" > > > > in the IDE. And IDEs also can reorganize the code when you save etc. > > > > We need a solid way to not be fighting the IDE all the time :). So it > may > > > > be okay to not go with how IDE formats things, but we need to ensure > IDE > > > > does not get in the way. > > > > > > > > thoughts? > > > > > > > > Thanks > > > > Vinoth > > > > > > > > On Fri, Aug 21, 2020 at 1:26 PM Nishith wrote: > > > > > > > > > +1 for spotless, automating the formatting will definitely help > > > > > productivity and turn around time for PRs. > > > > > > > > > > -Nishith > > > > > > > > > > Sent from my iPhone > > > > > > > > > > > On Aug 21, 2020, at 11:53 AM, Sivabalan > wrote: > > > > > > > > > > > > totally +1 for spotless. > > > > > > > > > > > > > > > > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf > wrote: > > > > > >> > > > > > >> +1 on using mvn spotless:apply to fix the codestyle. > > > > > >> > > > > > >> Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > > > > > >> > > > > > >>> +1 on auto code formatting. I also think it should be okay to be > > > even > > > > > >> more > > > > > >>> restrictive by failing builds when the code format is not adhered > > > (in > > > > > any > > > > > >>> environment). That way everyone is forced to use the same > > > formatting. > > > > > >>> > > > > > On Tue, Aug 18, 2020 at 8:47 PM vino yang < > [email protected]> > > > > > wrote: > > > > > >>> > > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > > > > >>> agreeing > > > > > on the same thing. > > > > > > > > > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, > if > > > it > > > > > >>> breaks > > > > > the code style, checkstyle will stop building and spotless will > > > > work. > > > > > > > > > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > > > > > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > > > > >>> agreeing > > > > > on > > > > > > the same thing. > > > > > > > > > > > > your understanding is correct. CI will enforce in a similar > > > > fashion. > > > > > > Spotless just makes us productive by auto fixing all the > > > checkstyle > > > > > > violations, without having to manually fix by hand. > > > > > > > > > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < > > > > > >> [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > >> I think adding spotless as a tooling command to auto fix code > is > > > > > > beneficial > > > > > >> and noth
Re: [DISCUSS] Codestyle: force multiline indentation
It can be up to the individual to use the IDE formatter or not, as long as there is a tool to help enforce Checkstyle rules. For people who use IDE formatter, importing Checkstyle.xml as a format scheme does not fully control the formatter's behavior, that's why IDE sometimes gets in the way. But most of the time it serves us well. Guess we can close the thread as we're all in favor of spotless? On Sat, Aug 22, 2020 at 6:08 AM vino yang wrote: > Hi vc, > > Yes, this part of the practice may have different preferences for different > developers. I have never opened the IDE's automatic formatting, nor have I > used the IDE's formatting functions artificially. Because I have > participated in multiple open source communities, each open source > community has its own conventions on code style. So, I just understand the > style of each community, after changing the code, and then compiling > locally, checkstyle will identify the related problems, and then report, > and then I will modify until the compilation is passed. > > I admit that this is my personal behavior, and everything has its two > sides. IDE automatic formatting will make it more convenient for developers > to deal with code styles. On the other hand, it will also make the > community more complicated when considering related conventions and weigh > more factors. > > Best, > Vino > > Vinoth Chandar 于2020年8月22日周六 下午2:25写道： > > > >But, IMO, we can ignore the IDE here, if it breaks the code style, > > checkstyle will stop building and spotless will work. > > > > I differ here slightly. Most people reformat code using the "format code" > > in the IDE. And IDEs also can reorganize the code when you save etc. > > We need a solid way to not be fighting the IDE all the time :). So it may > > be okay to not go with how IDE formats things, but we need to ensure IDE > > does not get in the way. > > > > thoughts? > > > > Thanks > > Vinoth > > > > On Fri, Aug 21, 2020 at 1:26 PM Nishith wrote: > > > > > +1 for spotless, automating the formatting will definitely help > > > productivity and turn around time for PRs. > > > > > > -Nishith > > > > > > Sent from my iPhone > > > > > > > On Aug 21, 2020, at 11:53 AM, Sivabalan wrote: > > > > > > > > totally +1 for spotless. > > > > > > > > > > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf wrote: > > > >> > > > >> +1 on using mvn spotless:apply to fix the codestyle. > > > >> > > > >> Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > > > >> > > > >>> +1 on auto code formatting. I also think it should be okay to be > even > > > >> more > > > >>> restrictive by failing builds when the code format is not adhered > (in > > > any > > > >>> environment). That way everyone is forced to use the same > formatting. > > > >>> > > > On Tue, Aug 18, 2020 at 8:47 PM vino yang > > > wrote: > > > >>> > > > > the key challenge has been keeping checkstyle, IDE and spotless > > > >>> agreeing > > > on the same thing. > > > > > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if > it > > > >>> breaks > > > the code style, checkstyle will stop building and spotless will > > work. > > > > > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > > >>> agreeing > > > on > > > > the same thing. > > > > > > > > your understanding is correct. CI will enforce in a similar > > fashion. > > > > Spotless just makes us productive by auto fixing all the > checkstyle > > > > violations, without having to manually fix by hand. > > > > > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < > > > >> [email protected] > > > > > > > wrote: > > > > > > > >> I think adding spotless as a tooling command to auto fix code is > > > > beneficial > > > >> and nothing harmful. > > > >> People are recommended to run it before commit or configure it > in > > a > > > >> pre-commit hook. > > > >> From the CI point of view, it does not change the existing way > of > > > > guarding > > > >> code style, does it? It'll still just run Checkstyle to report > > > >>> issues. > > > >> @Vinoth, am I understanding this correctly? Will Spotless be > based > > > >> on > > > the > > > >> same style configured via Checkstyle? > > > >> > > > >> On Tue, Aug 18, 2020 at 4:16 PM [email protected] < > > > >>> [email protected] > > > > > > > >> wrote: > > > >> > > > >>> +1 on standardizing code formatting. On Tuesday, August 18, > > > 2020, > > > >>> 03:58:42 PM PDT, Vinoth Chandar wrote: > > > >>> > > > >>> can more people please chime in? This will affect all of us on > > > >> a > > > > daily > > > >>> basis :) > > > >>> > > > >>> On Thu, Aug 13, 2020 at 8:25 AM Gary Li < > > > >> [email protected]> > > > >> wrote: > > > >>> > > > Vote for mvn spotless:apply to do the auto f
Re: [DISCUSS] Codestyle: force multiline indentation
Hi vc, Yes, this part of the practice may have different preferences for different developers. I have never opened the IDE's automatic formatting, nor have I used the IDE's formatting functions artificially. Because I have participated in multiple open source communities, each open source community has its own conventions on code style. So, I just understand the style of each community, after changing the code, and then compiling locally, checkstyle will identify the related problems, and then report, and then I will modify until the compilation is passed. I admit that this is my personal behavior, and everything has its two sides. IDE automatic formatting will make it more convenient for developers to deal with code styles. On the other hand, it will also make the community more complicated when considering related conventions and weigh more factors. Best, Vino Vinoth Chandar 于2020年8月22日周六 下午2:25写道： > >But, IMO, we can ignore the IDE here, if it breaks the code style, > checkstyle will stop building and spotless will work. > > I differ here slightly. Most people reformat code using the "format code" > in the IDE. And IDEs also can reorganize the code when you save etc. > We need a solid way to not be fighting the IDE all the time :). So it may > be okay to not go with how IDE formats things, but we need to ensure IDE > does not get in the way. > > thoughts? > > Thanks > Vinoth > > On Fri, Aug 21, 2020 at 1:26 PM Nishith wrote: > > > +1 for spotless, automating the formatting will definitely help > > productivity and turn around time for PRs. > > > > -Nishith > > > > Sent from my iPhone > > > > > On Aug 21, 2020, at 11:53 AM, Sivabalan wrote: > > > > > > totally +1 for spotless. > > > > > > > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf wrote: > > >> > > >> +1 on using mvn spotless:apply to fix the codestyle. > > >> > > >> Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > > >> > > >>> +1 on auto code formatting. I also think it should be okay to be even > > >> more > > >>> restrictive by failing builds when the code format is not adhered (in > > any > > >>> environment). That way everyone is forced to use the same formatting. > > >>> > > On Tue, Aug 18, 2020 at 8:47 PM vino yang > > wrote: > > >>> > > > the key challenge has been keeping checkstyle, IDE and spotless > > >>> agreeing > > on the same thing. > > > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it > > >>> breaks > > the code style, checkstyle will stop building and spotless will > work. > > > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > >>> agreeing > > on > > > the same thing. > > > > > > your understanding is correct. CI will enforce in a similar > fashion. > > > Spotless just makes us productive by auto fixing all the checkstyle > > > violations, without having to manually fix by hand. > > > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < > > >> [email protected] > > > > > wrote: > > > > > >> I think adding spotless as a tooling command to auto fix code is > > > beneficial > > >> and nothing harmful. > > >> People are recommended to run it before commit or configure it in > a > > >> pre-commit hook. > > >> From the CI point of view, it does not change the existing way of > > > guarding > > >> code style, does it? It'll still just run Checkstyle to report > > >>> issues. > > >> @Vinoth, am I understanding this correctly? Will Spotless be based > > >> on > > the > > >> same style configured via Checkstyle? > > >> > > >> On Tue, Aug 18, 2020 at 4:16 PM [email protected] < > > >>> [email protected] > > > > > >> wrote: > > >> > > >>> +1 on standardizing code formatting. On Tuesday, August 18, > > 2020, > > >>> 03:58:42 PM PDT, Vinoth Chandar wrote: > > >>> > > >>> can more people please chime in? This will affect all of us on > > >> a > > > daily > > >>> basis :) > > >>> > > >>> On Thu, Aug 13, 2020 at 8:25 AM Gary Li < > > >> [email protected]> > > >> wrote: > > >>> > > Vote for mvn spotless:apply to do the auto fix. > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar < > > >>> [email protected]> > > >>> wrote: > > > > > Hi, > > > > > > Anyone has thoughts on this? > > > > > > esp leesf/vinoyang, given you both drove much of the initial > > >> cleanups. > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > > >> [email protected] > > > > > wrote: > > > > > >> in that case, yes, all for automation. > > >> > > >> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > > > [email protected]> > > > wrote: > > >> > > >>> Overall, I think w
Re: [DISCUSS] Codestyle: force multiline indentation
>But, IMO, we can ignore the IDE here, if it breaks the code style, checkstyle will stop building and spotless will work. I differ here slightly. Most people reformat code using the "format code" in the IDE. And IDEs also can reorganize the code when you save etc. We need a solid way to not be fighting the IDE all the time :). So it may be okay to not go with how IDE formats things, but we need to ensure IDE does not get in the way. thoughts? Thanks Vinoth On Fri, Aug 21, 2020 at 1:26 PM Nishith wrote: > +1 for spotless, automating the formatting will definitely help > productivity and turn around time for PRs. > > -Nishith > > Sent from my iPhone > > > On Aug 21, 2020, at 11:53 AM, Sivabalan wrote: > > > > totally +1 for spotless. > > > > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf wrote: > >> > >> +1 on using mvn spotless:apply to fix the codestyle. > >> > >> Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > >> > >>> +1 on auto code formatting. I also think it should be okay to be even > >> more > >>> restrictive by failing builds when the code format is not adhered (in > any > >>> environment). That way everyone is forced to use the same formatting. > >>> > On Tue, Aug 18, 2020 at 8:47 PM vino yang > wrote: > >>> > > the key challenge has been keeping checkstyle, IDE and spotless > >>> agreeing > on the same thing. > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it > >>> breaks > the code style, checkstyle will stop building and spotless will work. > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > the key challenge has been keeping checkstyle, IDE and spotless > >>> agreeing > on > > the same thing. > > > > your understanding is correct. CI will enforce in a similar fashion. > > Spotless just makes us productive by auto fixing all the checkstyle > > violations, without having to manually fix by hand. > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < > >> [email protected] > > > wrote: > > > >> I think adding spotless as a tooling command to auto fix code is > > beneficial > >> and nothing harmful. > >> People are recommended to run it before commit or configure it in a > >> pre-commit hook. > >> From the CI point of view, it does not change the existing way of > > guarding > >> code style, does it? It'll still just run Checkstyle to report > >>> issues. > >> @Vinoth, am I understanding this correctly? Will Spotless be based > >> on > the > >> same style configured via Checkstyle? > >> > >> On Tue, Aug 18, 2020 at 4:16 PM [email protected] < > >>> [email protected] > > > >> wrote: > >> > >>> +1 on standardizing code formatting. On Tuesday, August 18, > 2020, > >>> 03:58:42 PM PDT, Vinoth Chandar wrote: > >>> > >>> can more people please chime in? This will affect all of us on > >> a > > daily > >>> basis :) > >>> > >>> On Thu, Aug 13, 2020 at 8:25 AM Gary Li < > >> [email protected]> > >> wrote: > >>> > Vote for mvn spotless:apply to do the auto fix. > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar < > >>> [email protected]> > >>> wrote: > > > Hi, > > > > Anyone has thoughts on this? > > > > esp leesf/vinoyang, given you both drove much of the initial > >> cleanups. > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > >> [email protected] > > > wrote: > > > >> in that case, yes, all for automation. > >> > >> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > > [email protected]> > > wrote: > >> > >>> Overall, I think we should standardize this across the > project. > >>> But most importantly, may be revive the long dormant > >>> spotless > >>> effort > >> first > >>> to enable autofixing of checkstyle issues, before we add > >>> more > checking? > >>> > >>> On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > [email protected] > >> > >>> wrote: > >>> > Hi all, > > I noticed that throughout the codebase, when method > arguments > >>> wrap > > to a > >>> new > line, there are cases where indentation is 4 and other > cases > >>> align > > the > wrapped line to the previous line of argument. > > The latter is caused by intelliJ settings of "Align > >> when > >>> multiline" > enabled. This won't be flagged by checkstyle due to not > > setting > *forceStrictCondition* to *true* > > > >>> > >> > > > > >>> > >> > > > > >>> > >> > https://chec
Re: [DISCUSS] Codestyle: force multiline indentation
+1 for spotless, automating the formatting will definitely help productivity and turn around time for PRs. -Nishith Sent from my iPhone > On Aug 21, 2020, at 11:53 AM, Sivabalan wrote: > > totally +1 for spotless. > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf wrote: >> >> +1 on using mvn spotless:apply to fix the codestyle. >> >> Bhavani Sudha 于2020年8月19日周三 下午12:31写道： >> >>> +1 on auto code formatting. I also think it should be okay to be even >> more >>> restrictive by failing builds when the code format is not adhered (in any >>> environment). That way everyone is forced to use the same formatting. >>> On Tue, Aug 18, 2020 at 8:47 PM vino yang wrote: >>> > the key challenge has been keeping checkstyle, IDE and spotless >>> agreeing on the same thing. Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it >>> breaks the code style, checkstyle will stop building and spotless will work. Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > the key challenge has been keeping checkstyle, IDE and spotless >>> agreeing on > the same thing. > > your understanding is correct. CI will enforce in a similar fashion. > Spotless just makes us productive by auto fixing all the checkstyle > violations, without having to manually fix by hand. > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < >> [email protected] > wrote: > >> I think adding spotless as a tooling command to auto fix code is > beneficial >> and nothing harmful. >> People are recommended to run it before commit or configure it in a >> pre-commit hook. >> From the CI point of view, it does not change the existing way of > guarding >> code style, does it? It'll still just run Checkstyle to report >>> issues. >> @Vinoth, am I understanding this correctly? Will Spotless be based >> on the >> same style configured via Checkstyle? >> >> On Tue, Aug 18, 2020 at 4:16 PM [email protected] < >>> [email protected] > >> wrote: >> >>> +1 on standardizing code formatting. On Tuesday, August 18, 2020, >>> 03:58:42 PM PDT, Vinoth Chandar wrote: >>> >>> can more people please chime in? This will affect all of us on >> a > daily >>> basis :) >>> >>> On Thu, Aug 13, 2020 at 8:25 AM Gary Li < >> [email protected]> >> wrote: >>> Vote for mvn spotless:apply to do the auto fix. On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar < >>> [email protected]> >>> wrote: > Hi, > > Anyone has thoughts on this? > > esp leesf/vinoyang, given you both drove much of the initial >> cleanups. > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < >> [email protected] > wrote: > >> in that case, yes, all for automation. >> >> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > [email protected]> > wrote: >> >>> Overall, I think we should standardize this across the project. >>> But most importantly, may be revive the long dormant >>> spotless >>> effort >> first >>> to enable autofixing of checkstyle issues, before we add >>> more checking? >>> >>> On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < [email protected] >> >>> wrote: >>> Hi all, I noticed that throughout the codebase, when method arguments >>> wrap > to a >>> new line, there are cases where indentation is 4 and other cases >>> align > the wrapped line to the previous line of argument. The latter is caused by intelliJ settings of "Align >> when >>> multiline" enabled. This won't be flagged by checkstyle due to not > setting *forceStrictCondition* to *true* >>> >> > >>> >> > >>> >> https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties I'm suggesting setting this to true to avoid the discrepancy >> and >>> redundant diffs in PR caused by individual IDE settings. People >> who > have >>> set >> "Align when multiline" will need to disable it to pass the > checkstyle >>> validation. WDYT? Best, Raymond >>> >> > >>> >> > >>> >> > > > -- > Regards, > -Sivabalan
Re: [DISCUSS] Codestyle: force multiline indentation
totally +1 for spotless. On Thu, Aug 20, 2020 at 8:53 AM leesf wrote: > +1 on using mvn spotless:apply to fix the codestyle. > > Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > > > +1 on auto code formatting. I also think it should be okay to be even > more > > restrictive by failing builds when the code format is not adhered (in any > > environment). That way everyone is forced to use the same formatting. > > > > On Tue, Aug 18, 2020 at 8:47 PM vino yang wrote: > > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > agreeing > > > on the same thing. > > > > > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it > > breaks > > > the code style, checkstyle will stop building and spotless will work. > > > > > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > > > > > the key challenge has been keeping checkstyle, IDE and spotless > > agreeing > > > on > > > > the same thing. > > > > > > > > your understanding is correct. CI will enforce in a similar fashion. > > > > Spotless just makes us productive by auto fixing all the checkstyle > > > > violations, without having to manually fix by hand. > > > > > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu < > [email protected] > > > > > > > wrote: > > > > > > > > > I think adding spotless as a tooling command to auto fix code is > > > > beneficial > > > > > and nothing harmful. > > > > > People are recommended to run it before commit or configure it in a > > > > > pre-commit hook. > > > > > From the CI point of view, it does not change the existing way of > > > > guarding > > > > > code style, does it? It'll still just run Checkstyle to report > > issues. > > > > > @Vinoth, am I understanding this correctly? Will Spotless be based > on > > > the > > > > > same style configured via Checkstyle? > > > > > > > > > > On Tue, Aug 18, 2020 at 4:16 PM [email protected] < > > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > +1 on standardizing code formatting. On Tuesday, August 18, > > > 2020, > > > > > > 03:58:42 PM PDT, Vinoth Chandar wrote: > > > > > > > > > > > > can more people please chime in? This will affect all of us on > a > > > > daily > > > > > > basis :) > > > > > > > > > > > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li < > [email protected]> > > > > > wrote: > > > > > > > > > > > > > Vote for mvn spotless:apply to do the auto fix. > > > > > > > > > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar < > > [email protected]> > > > > > > wrote: > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > Anyone has thoughts on this? > > > > > > > > > > > > > > > > esp leesf/vinoyang, given you both drove much of the initial > > > > > cleanups. > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > > > > > [email protected] > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > in that case, yes, all for automation. > > > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > > > > [email protected]> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Overall, I think we should standardize this across the > > > project. > > > > > > > > > > But most importantly, may be revive the long dormant > > spotless > > > > > > effort > > > > > > > > > first > > > > > > > > > > to enable autofixing of checkstyle issues, before we add > > more > > > > > > > checking? > > > > > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > > > I noticed that throughout the codebase, when method > > > arguments > > > > > > wrap > > > > > > > > to a > > > > > > > > > > new > > > > > > > > > > > line, there are cases where indentation is 4 and other > > > cases > > > > > > align > > > > > > > > the > > > > > > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align > when > > > > > > multiline" > > > > > > > > > > > enabled. This won't be flagged by checkstyle due to not > > > > setting > > > > > > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > > > > > > > > > > > I'm suggesting setting this to true to avoid the > > > discrepancy > > > > > and > > > > > > > > > > redundant > > > > > > > > > > > diffs in PR caused by individual IDE settings. People > who > > > > have > > > > > > set > > > > > > > > > "Align > > > > > > > > > > > when multiline" will need to disable it to pass the > > > > checkstyle > > > > > > > > > > validation. > > > > > > > > > > > > > > > > > > > > > > WDYT? > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
+1 on using mvn spotless:apply to fix the codestyle. Bhavani Sudha 于2020年8月19日周三 下午12:31写道： > +1 on auto code formatting. I also think it should be okay to be even more > restrictive by failing builds when the code format is not adhered (in any > environment). That way everyone is forced to use the same formatting. > > On Tue, Aug 18, 2020 at 8:47 PM vino yang wrote: > > > > the key challenge has been keeping checkstyle, IDE and spotless > agreeing > > on the same thing. > > > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it > breaks > > the code style, checkstyle will stop building and spotless will work. > > > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > > > the key challenge has been keeping checkstyle, IDE and spotless > agreeing > > on > > > the same thing. > > > > > > your understanding is correct. CI will enforce in a similar fashion. > > > Spotless just makes us productive by auto fixing all the checkstyle > > > violations, without having to manually fix by hand. > > > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu > > > > wrote: > > > > > > > I think adding spotless as a tooling command to auto fix code is > > > beneficial > > > > and nothing harmful. > > > > People are recommended to run it before commit or configure it in a > > > > pre-commit hook. > > > > From the CI point of view, it does not change the existing way of > > > guarding > > > > code style, does it? It'll still just run Checkstyle to report > issues. > > > > @Vinoth, am I understanding this correctly? Will Spotless be based on > > the > > > > same style configured via Checkstyle? > > > > > > > > On Tue, Aug 18, 2020 at 4:16 PM [email protected] < > [email protected] > > > > > > > wrote: > > > > > > > > > +1 on standardizing code formatting. On Tuesday, August 18, > > 2020, > > > > > 03:58:42 PM PDT, Vinoth Chandar wrote: > > > > > > > > > > can more people please chime in? This will affect all of us on a > > > daily > > > > > basis :) > > > > > > > > > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li > > > > wrote: > > > > > > > > > > > Vote for mvn spotless:apply to do the auto fix. > > > > > > > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar < > [email protected]> > > > > > wrote: > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > Anyone has thoughts on this? > > > > > > > > > > > > > > esp leesf/vinoyang, given you both drove much of the initial > > > > cleanups. > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > > > > [email protected] > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > in that case, yes, all for automation. > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > > > Overall, I think we should standardize this across the > > project. > > > > > > > > > But most importantly, may be revive the long dormant > spotless > > > > > effort > > > > > > > > first > > > > > > > > > to enable autofixing of checkstyle issues, before we add > more > > > > > > checking? > > > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > > > > > [email protected] > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > > > I noticed that throughout the codebase, when method > > arguments > > > > > wrap > > > > > > > to a > > > > > > > > > new > > > > > > > > > > line, there are cases where indentation is 4 and other > > cases > > > > > align > > > > > > > the > > > > > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align when > > > > > multiline" > > > > > > > > > > enabled. This won't be flagged by checkstyle due to not > > > setting > > > > > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > > > > > > > > > I'm suggesting setting this to true to avoid the > > discrepancy > > > > and > > > > > > > > > redundant > > > > > > > > > > diffs in PR caused by individual IDE settings. People who > > > have > > > > > set > > > > > > > > "Align > > > > > > > > > > when multiline" will need to disable it to pass the > > > checkstyle > > > > > > > > > validation. > > > > > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > Raymond > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
+1 on auto code formatting. I also think it should be okay to be even more restrictive by failing builds when the code format is not adhered (in any environment). That way everyone is forced to use the same formatting. On Tue, Aug 18, 2020 at 8:47 PM vino yang wrote: > > the key challenge has been keeping checkstyle, IDE and spotless agreeing > on the same thing. > > Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it breaks > the code style, checkstyle will stop building and spotless will work. > > Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > > > the key challenge has been keeping checkstyle, IDE and spotless agreeing > on > > the same thing. > > > > your understanding is correct. CI will enforce in a similar fashion. > > Spotless just makes us productive by auto fixing all the checkstyle > > violations, without having to manually fix by hand. > > > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu > > wrote: > > > > > I think adding spotless as a tooling command to auto fix code is > > beneficial > > > and nothing harmful. > > > People are recommended to run it before commit or configure it in a > > > pre-commit hook. > > > From the CI point of view, it does not change the existing way of > > guarding > > > code style, does it? It'll still just run Checkstyle to report issues. > > > @Vinoth, am I understanding this correctly? Will Spotless be based on > the > > > same style configured via Checkstyle? > > > > > > On Tue, Aug 18, 2020 at 4:16 PM [email protected] > > > > wrote: > > > > > > > +1 on standardizing code formatting. On Tuesday, August 18, > 2020, > > > > 03:58:42 PM PDT, Vinoth Chandar wrote: > > > > > > > > can more people please chime in? This will affect all of us on a > > daily > > > > basis :) > > > > > > > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li > > > wrote: > > > > > > > > > Vote for mvn spotless:apply to do the auto fix. > > > > > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > Anyone has thoughts on this? > > > > > > > > > > > > esp leesf/vinoyang, given you both drove much of the initial > > > cleanups. > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > > > in that case, yes, all for automation. > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > > [email protected]> > > > > > > wrote: > > > > > > > > > > > > > > > Overall, I think we should standardize this across the > project. > > > > > > > > But most importantly, may be revive the long dormant spotless > > > > effort > > > > > > > first > > > > > > > > to enable autofixing of checkstyle issues, before we add more > > > > > checking? > > > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > > > > [email protected] > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > I noticed that throughout the codebase, when method > arguments > > > > wrap > > > > > > to a > > > > > > > > new > > > > > > > > > line, there are cases where indentation is 4 and other > cases > > > > align > > > > > > the > > > > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align when > > > > multiline" > > > > > > > > > enabled. This won't be flagged by checkstyle due to not > > setting > > > > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > > > > > > > I'm suggesting setting this to true to avoid the > discrepancy > > > and > > > > > > > > redundant > > > > > > > > > diffs in PR caused by individual IDE settings. People who > > have > > > > set > > > > > > > "Align > > > > > > > > > when multiline" will need to disable it to pass the > > checkstyle > > > > > > > > validation. > > > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Raymond > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
> the key challenge has been keeping checkstyle, IDE and spotless agreeing on the same thing. Yes, it's the key thing. But, IMO, we can ignore the IDE here, if it breaks the code style, checkstyle will stop building and spotless will work. Vinoth Chandar 于2020年8月19日周三 上午7:49写道： > the key challenge has been keeping checkstyle, IDE and spotless agreeing on > the same thing. > > your understanding is correct. CI will enforce in a similar fashion. > Spotless just makes us productive by auto fixing all the checkstyle > violations, without having to manually fix by hand. > > On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu > wrote: > > > I think adding spotless as a tooling command to auto fix code is > beneficial > > and nothing harmful. > > People are recommended to run it before commit or configure it in a > > pre-commit hook. > > From the CI point of view, it does not change the existing way of > guarding > > code style, does it? It'll still just run Checkstyle to report issues. > > @Vinoth, am I understanding this correctly? Will Spotless be based on the > > same style configured via Checkstyle? > > > > On Tue, Aug 18, 2020 at 4:16 PM [email protected] > > wrote: > > > > > +1 on standardizing code formatting. On Tuesday, August 18, 2020, > > > 03:58:42 PM PDT, Vinoth Chandar wrote: > > > > > > can more people please chime in? This will affect all of us on a > daily > > > basis :) > > > > > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li > > wrote: > > > > > > > Vote for mvn spotless:apply to do the auto fix. > > > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > Anyone has thoughts on this? > > > > > > > > > > esp leesf/vinoyang, given you both drove much of the initial > > cleanups. > > > > > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > in that case, yes, all for automation. > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar < > [email protected]> > > > > > wrote: > > > > > > > > > > > > > Overall, I think we should standardize this across the project. > > > > > > > But most importantly, may be revive the long dormant spotless > > > effort > > > > > > first > > > > > > > to enable autofixing of checkstyle issues, before we add more > > > > checking? > > > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > > > [email protected] > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I noticed that throughout the codebase, when method arguments > > > wrap > > > > > to a > > > > > > > new > > > > > > > > line, there are cases where indentation is 4 and other cases > > > align > > > > > the > > > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align when > > > multiline" > > > > > > > > enabled. This won't be flagged by checkstyle due to not > setting > > > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy > > and > > > > > > > redundant > > > > > > > > diffs in PR caused by individual IDE settings. People who > have > > > set > > > > > > "Align > > > > > > > > when multiline" will need to disable it to pass the > checkstyle > > > > > > > validation. > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > Best, > > > > > > > > Raymond > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
the key challenge has been keeping checkstyle, IDE and spotless agreeing on the same thing. your understanding is correct. CI will enforce in a similar fashion. Spotless just makes us productive by auto fixing all the checkstyle violations, without having to manually fix by hand. On Tue, Aug 18, 2020 at 4:42 PM Shiyan Xu wrote: > I think adding spotless as a tooling command to auto fix code is beneficial > and nothing harmful. > People are recommended to run it before commit or configure it in a > pre-commit hook. > From the CI point of view, it does not change the existing way of guarding > code style, does it? It'll still just run Checkstyle to report issues. > @Vinoth, am I understanding this correctly? Will Spotless be based on the > same style configured via Checkstyle? > > On Tue, Aug 18, 2020 at 4:16 PM [email protected] > wrote: > > > +1 on standardizing code formatting. On Tuesday, August 18, 2020, > > 03:58:42 PM PDT, Vinoth Chandar wrote: > > > > can more people please chime in? This will affect all of us on a daily > > basis :) > > > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li > wrote: > > > > > Vote for mvn spotless:apply to do the auto fix. > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar > > wrote: > > > > > > > Hi, > > > > > > > > Anyone has thoughts on this? > > > > > > > > esp leesf/vinoyang, given you both drove much of the initial > cleanups. > > > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu < > [email protected] > > > > > > > wrote: > > > > > > > > > in that case, yes, all for automation. > > > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar > > > > wrote: > > > > > > > > > > > Overall, I think we should standardize this across the project. > > > > > > But most importantly, may be revive the long dormant spotless > > effort > > > > > first > > > > > > to enable autofixing of checkstyle issues, before we add more > > > checking? > > > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > I noticed that throughout the codebase, when method arguments > > wrap > > > > to a > > > > > > new > > > > > > > line, there are cases where indentation is 4 and other cases > > align > > > > the > > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align when > > multiline" > > > > > > > enabled. This won't be flagged by checkstyle due to not setting > > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy > and > > > > > > redundant > > > > > > > diffs in PR caused by individual IDE settings. People who have > > set > > > > > "Align > > > > > > > when multiline" will need to disable it to pass the checkstyle > > > > > > validation. > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > Best, > > > > > > > Raymond > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
I think adding spotless as a tooling command to auto fix code is beneficial and nothing harmful. People are recommended to run it before commit or configure it in a pre-commit hook. >From the CI point of view, it does not change the existing way of guarding code style, does it? It'll still just run Checkstyle to report issues. @Vinoth, am I understanding this correctly? Will Spotless be based on the same style configured via Checkstyle? On Tue, Aug 18, 2020 at 4:16 PM [email protected] wrote: > +1 on standardizing code formatting. On Tuesday, August 18, 2020, > 03:58:42 PM PDT, Vinoth Chandar wrote: > > can more people please chime in? This will affect all of us on a daily > basis :) > > On Thu, Aug 13, 2020 at 8:25 AM Gary Li wrote: > > > Vote for mvn spotless:apply to do the auto fix. > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar > wrote: > > > > > Hi, > > > > > > Anyone has thoughts on this? > > > > > > esp leesf/vinoyang, given you both drove much of the initial cleanups. > > > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu > > > > wrote: > > > > > > > in that case, yes, all for automation. > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar > > > wrote: > > > > > > > > > Overall, I think we should standardize this across the project. > > > > > But most importantly, may be revive the long dormant spotless > effort > > > > first > > > > > to enable autofixing of checkstyle issues, before we add more > > checking? > > > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > > [email protected] > > > > > > > > > wrote: > > > > > > > > > > > Hi all, > > > > > > > > > > > > I noticed that throughout the codebase, when method arguments > wrap > > > to a > > > > > new > > > > > > line, there are cases where indentation is 4 and other cases > align > > > the > > > > > > wrapped line to the previous line of argument. > > > > > > > > > > > > The latter is caused by intelliJ settings of "Align when > multiline" > > > > > > enabled. This won't be flagged by checkstyle due to not setting > > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy and > > > > > redundant > > > > > > diffs in PR caused by individual IDE settings. People who have > set > > > > "Align > > > > > > when multiline" will need to disable it to pass the checkstyle > > > > > validation. > > > > > > > > > > > > WDYT? > > > > > > > > > > > > Best, > > > > > > Raymond > > > > > > > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
+1 on standardizing code formatting. On Tuesday, August 18, 2020, 03:58:42 PM PDT, Vinoth Chandar wrote: can more people please chime in? This will affect all of us on a daily basis :) On Thu, Aug 13, 2020 at 8:25 AM Gary Li wrote: > Vote for mvn spotless:apply to do the auto fix. > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar wrote: > > > Hi, > > > > Anyone has thoughts on this? > > > > esp leesf/vinoyang, given you both drove much of the initial cleanups. > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu > > wrote: > > > > > in that case, yes, all for automation. > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar > > wrote: > > > > > > > Overall, I think we should standardize this across the project. > > > > But most importantly, may be revive the long dormant spotless effort > > > first > > > > to enable autofixing of checkstyle issues, before we add more > checking? > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > [email protected] > > > > > > > wrote: > > > > > > > > > Hi all, > > > > > > > > > > I noticed that throughout the codebase, when method arguments wrap > > to a > > > > new > > > > > line, there are cases where indentation is 4 and other cases align > > the > > > > > wrapped line to the previous line of argument. > > > > > > > > > > The latter is caused by intelliJ settings of "Align when multiline" > > > > > enabled. This won't be flagged by checkstyle due to not setting > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy and > > > > redundant > > > > > diffs in PR caused by individual IDE settings. People who have set > > > "Align > > > > > when multiline" will need to disable it to pass the checkstyle > > > > validation. > > > > > > > > > > WDYT? > > > > > > > > > > Best, > > > > > Raymond > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
can more people please chime in? This will affect all of us on a daily basis :) On Thu, Aug 13, 2020 at 8:25 AM Gary Li wrote: > Vote for mvn spotless:apply to do the auto fix. > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar wrote: > > > Hi, > > > > Anyone has thoughts on this? > > > > esp leesf/vinoyang, given you both drove much of the initial cleanups. > > > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu > > wrote: > > > > > in that case, yes, all for automation. > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar > > wrote: > > > > > > > Overall, I think we should standardize this across the project. > > > > But most importantly, may be revive the long dormant spotless effort > > > first > > > > to enable autofixing of checkstyle issues, before we add more > checking? > > > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu < > [email protected] > > > > > > > wrote: > > > > > > > > > Hi all, > > > > > > > > > > I noticed that throughout the codebase, when method arguments wrap > > to a > > > > new > > > > > line, there are cases where indentation is 4 and other cases align > > the > > > > > wrapped line to the previous line of argument. > > > > > > > > > > The latter is caused by intelliJ settings of "Align when multiline" > > > > > enabled. This won't be flagged by checkstyle due to not setting > > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy and > > > > redundant > > > > > diffs in PR caused by individual IDE settings. People who have set > > > "Align > > > > > when multiline" will need to disable it to pass the checkstyle > > > > validation. > > > > > > > > > > WDYT? > > > > > > > > > > Best, > > > > > Raymond > > > > > > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
Vote for mvn spotless:apply to do the auto fix. On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar wrote: > Hi, > > Anyone has thoughts on this? > > esp leesf/vinoyang, given you both drove much of the initial cleanups. > > On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu > wrote: > > > in that case, yes, all for automation. > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar > wrote: > > > > > Overall, I think we should standardize this across the project. > > > But most importantly, may be revive the long dormant spotless effort > > first > > > to enable autofixing of checkstyle issues, before we add more checking? > > > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu > > > > wrote: > > > > > > > Hi all, > > > > > > > > I noticed that throughout the codebase, when method arguments wrap > to a > > > new > > > > line, there are cases where indentation is 4 and other cases align > the > > > > wrapped line to the previous line of argument. > > > > > > > > The latter is caused by intelliJ settings of "Align when multiline" > > > > enabled. This won't be flagged by checkstyle due to not setting > > > > *forceStrictCondition* to *true* > > > > > > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > > > I'm suggesting setting this to true to avoid the discrepancy and > > > redundant > > > > diffs in PR caused by individual IDE settings. People who have set > > "Align > > > > when multiline" will need to disable it to pass the checkstyle > > > validation. > > > > > > > > WDYT? > > > > > > > > Best, > > > > Raymond > > > > > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
Hi, Anyone has thoughts on this? esp leesf/vinoyang, given you both drove much of the initial cleanups. On Mon, Aug 10, 2020 at 7:16 PM Shiyan Xu wrote: > in that case, yes, all for automation. > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar wrote: > > > Overall, I think we should standardize this across the project. > > But most importantly, may be revive the long dormant spotless effort > first > > to enable autofixing of checkstyle issues, before we add more checking? > > > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu > > wrote: > > > > > Hi all, > > > > > > I noticed that throughout the codebase, when method arguments wrap to a > > new > > > line, there are cases where indentation is 4 and other cases align the > > > wrapped line to the previous line of argument. > > > > > > The latter is caused by intelliJ settings of "Align when multiline" > > > enabled. This won't be flagged by checkstyle due to not setting > > > *forceStrictCondition* to *true* > > > > > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > > > I'm suggesting setting this to true to avoid the discrepancy and > > redundant > > > diffs in PR caused by individual IDE settings. People who have set > "Align > > > when multiline" will need to disable it to pass the checkstyle > > validation. > > > > > > WDYT? > > > > > > Best, > > > Raymond > > > > > >
Re: [DISCUSS] Codestyle: force multiline indentation
in that case, yes, all for automation. On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar wrote: > Overall, I think we should standardize this across the project. > But most importantly, may be revive the long dormant spotless effort first > to enable autofixing of checkstyle issues, before we add more checking? > > On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu > wrote: > > > Hi all, > > > > I noticed that throughout the codebase, when method arguments wrap to a > new > > line, there are cases where indentation is 4 and other cases align the > > wrapped line to the previous line of argument. > > > > The latter is caused by intelliJ settings of "Align when multiline" > > enabled. This won't be flagged by checkstyle due to not setting > > *forceStrictCondition* to *true* > > > > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > > > I'm suggesting setting this to true to avoid the discrepancy and > redundant > > diffs in PR caused by individual IDE settings. People who have set "Align > > when multiline" will need to disable it to pass the checkstyle > validation. > > > > WDYT? > > > > Best, > > Raymond > > >
Re: [DISCUSS] Codestyle: force multiline indentation
Overall, I think we should standardize this across the project. But most importantly, may be revive the long dormant spotless effort first to enable autofixing of checkstyle issues, before we add more checking? On Mon, Aug 10, 2020 at 7:04 PM Shiyan Xu wrote: > Hi all, > > I noticed that throughout the codebase, when method arguments wrap to a new > line, there are cases where indentation is 4 and other cases align the > wrapped line to the previous line of argument. > > The latter is caused by intelliJ settings of "Align when multiline" > enabled. This won't be flagged by checkstyle due to not setting > *forceStrictCondition* to *true* > > https://checkstyle.sourceforge.io/config_misc.html#Indentation_Properties > > I'm suggesting setting this to true to avoid the discrepancy and redundant > diffs in PR caused by individual IDE settings. People who have set "Align > when multiline" will need to disable it to pass the checkstyle validation. > > WDYT? > > Best, > Raymond >
