You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hudi.apache.org by Shiyan Xu <xu...@gmail.com> on 2020/08/11 02:04:21 UTC
[DISCUSS] Codestyle: force multiline indentation
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
Posted by Vinoth Chandar <vi...@apache.org>.
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 <xu...@gmail.com>
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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <n3...@gmail.com> 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 <n....@gmail.com>
> wrote:
>
> > > > >
>
> > > > > totally +1 for spotless.
>
> > > > >
>
> > > > >
>
> > > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com>
> wrote:
>
> > > > >>
>
> > > > >> +1 on using mvn spotless:apply to fix the codestyle.
>
> > > > >>
>
> > > > >> Bhavani Sudha <bh...@gmail.com> 于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 <
> yanghua1127@gmail.com>
>
> > > > 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 <vi...@apache.org> 于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 <
>
> > > > >> xu.shiyan.raymond@gmail.com
>
> > > > >>>>
>
> > > > >>>>> 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 vbalaji@apache.org <
>
> > > > >>> vbalaji@apache.org
>
> > > > >>>>>
>
> > > > >>>>>> wrote:
>
> > > > >>>>>>
>
> > > > >>>>>>> +1 on standardizing code formatting. On Tuesday, August
> 18,
>
> > > > >>>> 2020,
>
> > > > >>>>>>> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
>
> > > > >> yanjia.gary.li@gmail.com>
>
> > > > >>>>>> wrote:
>
> > > > >>>>>>>
>
> > > > >>>>>>>> Vote for mvn spotless:apply to do the auto fix.
>
> > > > >>>>>>>>
>
> > > > >>>>>>>> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
>
> > > > >>> vinoth@apache.org>
>
> > > > >>>>>>> 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 <
>
> > > > >>>>>> xu.shiyan.raymond@gmail.com
>
> > > > >>>>>>>>
>
> > > > >>>>>>>>> wrote:
>
> > > > >>>>>>>>>
>
> > > > >>>>>>>>>> in that case, yes, all for automation.
>
> > > > >>>>>>>>>>
>
> > > > >>>>>>>>>> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
>
> > > > >>>>> vinoth@apache.org>
>
> > > > >>>>>>>>> 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 <
>
> > > > >>>>>>>> xu.shiyan.raymond@gmail.com
>
> > > > >>>>>>>>>>
>
> > > > >>>>>>>>>>> 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
Posted by Shiyan Xu <xu...@gmail.com>.
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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <n3...@gmail.com> 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 <n....@gmail.com> wrote:
> > > >
> > > > totally +1 for spotless.
> > > >
> > > >
> > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com> wrote:
> > > >>
> > > >> +1 on using mvn spotless:apply to fix the codestyle.
> > > >>
> > > >> Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com>
> > > 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 <vi...@apache.org> 于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 <
> > > >> xu.shiyan.raymond@gmail.com
> > > >>>>
> > > >>>>> 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 vbalaji@apache.org <
> > > >>> vbalaji@apache.org
> > > >>>>>
> > > >>>>>> wrote:
> > > >>>>>>
> > > >>>>>>> +1 on standardizing code formatting. On Tuesday, August 18,
> > > >>>> 2020,
> > > >>>>>>> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
> > > >> yanjia.gary.li@gmail.com>
> > > >>>>>> wrote:
> > > >>>>>>>
> > > >>>>>>>> Vote for mvn spotless:apply to do the auto fix.
> > > >>>>>>>>
> > > >>>>>>>> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
> > > >>> vinoth@apache.org>
> > > >>>>>>> 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 <
> > > >>>>>> xu.shiyan.raymond@gmail.com
> > > >>>>>>>>
> > > >>>>>>>>> wrote:
> > > >>>>>>>>>
> > > >>>>>>>>>> in that case, yes, all for automation.
> > > >>>>>>>>>>
> > > >>>>>>>>>> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> > > >>>>> vinoth@apache.org>
> > > >>>>>>>>> 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 <
> > > >>>>>>>> xu.shiyan.raymond@gmail.com
> > > >>>>>>>>>>
> > > >>>>>>>>>>> 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
Posted by vino yang <ya...@gmail.com>.
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 <vi...@apache.org> 于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 <n3...@gmail.com> 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 <n....@gmail.com> wrote:
> > >
> > > totally +1 for spotless.
> > >
> > >
> > >> On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com> wrote:
> > >>
> > >> +1 on using mvn spotless:apply to fix the codestyle.
> > >>
> > >> Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com>
> > 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 <vi...@apache.org> 于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 <
> > >> xu.shiyan.raymond@gmail.com
> > >>>>
> > >>>>> 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 vbalaji@apache.org <
> > >>> vbalaji@apache.org
> > >>>>>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> +1 on standardizing code formatting. On Tuesday, August 18,
> > >>>> 2020,
> > >>>>>>> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
> > >> yanjia.gary.li@gmail.com>
> > >>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> Vote for mvn spotless:apply to do the auto fix.
> > >>>>>>>>
> > >>>>>>>> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
> > >>> vinoth@apache.org>
> > >>>>>>> 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 <
> > >>>>>> xu.shiyan.raymond@gmail.com
> > >>>>>>>>
> > >>>>>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>>> in that case, yes, all for automation.
> > >>>>>>>>>>
> > >>>>>>>>>> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> > >>>>> vinoth@apache.org>
> > >>>>>>>>> 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 <
> > >>>>>>>> xu.shiyan.raymond@gmail.com
> > >>>>>>>>>>
> > >>>>>>>>>>> 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
Posted by Vinoth Chandar <vi...@apache.org>.
>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 <n3...@gmail.com> 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 <n....@gmail.com> wrote:
> >
> > totally +1 for spotless.
> >
> >
> >> On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com> wrote:
> >>
> >> +1 on using mvn spotless:apply to fix the codestyle.
> >>
> >> Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com>
> 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 <vi...@apache.org> 于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 <
> >> xu.shiyan.raymond@gmail.com
> >>>>
> >>>>> 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 vbalaji@apache.org <
> >>> vbalaji@apache.org
> >>>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> +1 on standardizing code formatting. On Tuesday, August 18,
> >>>> 2020,
> >>>>>>> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
> >> yanjia.gary.li@gmail.com>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>>> Vote for mvn spotless:apply to do the auto fix.
> >>>>>>>>
> >>>>>>>> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
> >>> vinoth@apache.org>
> >>>>>>> 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 <
> >>>>>> xu.shiyan.raymond@gmail.com
> >>>>>>>>
> >>>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> in that case, yes, all for automation.
> >>>>>>>>>>
> >>>>>>>>>> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> >>>>> vinoth@apache.org>
> >>>>>>>>> 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 <
> >>>>>>>> xu.shiyan.raymond@gmail.com
> >>>>>>>>>>
> >>>>>>>>>>> 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
Posted by Nishith <n3...@gmail.com>.
+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 <n....@gmail.com> wrote:
>
> totally +1 for spotless.
>
>
>> On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com> wrote:
>>
>> +1 on using mvn spotless:apply to fix the codestyle.
>>
>> Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <
>> xu.shiyan.raymond@gmail.com
>>>>
>>>>> 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 vbalaji@apache.org <
>>> vbalaji@apache.org
>>>>>
>>>>>> wrote:
>>>>>>
>>>>>>> +1 on standardizing code formatting. On Tuesday, August 18,
>>>> 2020,
>>>>>>> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
>> yanjia.gary.li@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>> Vote for mvn spotless:apply to do the auto fix.
>>>>>>>>
>>>>>>>> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
>>> vinoth@apache.org>
>>>>>>> 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 <
>>>>>> xu.shiyan.raymond@gmail.com
>>>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> in that case, yes, all for automation.
>>>>>>>>>>
>>>>>>>>>> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
>>>>> vinoth@apache.org>
>>>>>>>>> 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 <
>>>>>>>> xu.shiyan.raymond@gmail.com
>>>>>>>>>>
>>>>>>>>>>> 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
Posted by Sivabalan <n....@gmail.com>.
totally +1 for spotless.
On Thu, Aug 20, 2020 at 8:53 AM leesf <le...@gmail.com> wrote:
> +1 on using mvn spotless:apply to fix the codestyle.
>
> Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <
> xu.shiyan.raymond@gmail.com
> > >
> > > > 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 vbalaji@apache.org <
> > vbalaji@apache.org
> > > >
> > > > > wrote:
> > > > >
> > > > > > +1 on standardizing code formatting. On Tuesday, August 18,
> > > 2020,
> > > > > > 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <
> yanjia.gary.li@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > Vote for mvn spotless:apply to do the auto fix.
> > > > > > >
> > > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
> > vinoth@apache.org>
> > > > > > 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 <
> > > > > xu.shiyan.raymond@gmail.com
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > in that case, yes, all for automation.
> > > > > > > > >
> > > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> > > > vinoth@apache.org>
> > > > > > > > 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 <
> > > > > > > xu.shiyan.raymond@gmail.com
> > > > > > > > >
> > > > > > > > > > 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
Posted by leesf <le...@gmail.com>.
+1 on using mvn spotless:apply to fix the codestyle.
Bhavani Sudha <bh...@gmail.com> 于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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <xu.shiyan.raymond@gmail.com
> >
> > > 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 vbalaji@apache.org <
> vbalaji@apache.org
> > >
> > > > wrote:
> > > >
> > > > > +1 on standardizing code formatting. On Tuesday, August 18,
> > 2020,
> > > > > 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com>
> > > > wrote:
> > > > >
> > > > > > Vote for mvn spotless:apply to do the auto fix.
> > > > > >
> > > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <
> vinoth@apache.org>
> > > > > 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 <
> > > > xu.shiyan.raymond@gmail.com
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > in that case, yes, all for automation.
> > > > > > > >
> > > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> > > vinoth@apache.org>
> > > > > > > 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 <
> > > > > > xu.shiyan.raymond@gmail.com
> > > > > > > >
> > > > > > > > > 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
Posted by Bhavani Sudha <bh...@gmail.com>.
+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 <ya...@gmail.com> 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 <vi...@apache.org> 于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 <xu...@gmail.com>
> > 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 vbalaji@apache.org <vbalaji@apache.org
> >
> > > wrote:
> > >
> > > > +1 on standardizing code formatting. On Tuesday, August 18,
> 2020,
> > > > 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com>
> > > wrote:
> > > >
> > > > > Vote for mvn spotless:apply to do the auto fix.
> > > > >
> > > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org>
> > > > 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 <
> > > xu.shiyan.raymond@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > in that case, yes, all for automation.
> > > > > > >
> > > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> > vinoth@apache.org>
> > > > > > 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 <
> > > > > xu.shiyan.raymond@gmail.com
> > > > > > >
> > > > > > > > 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
Posted by vino yang <ya...@gmail.com>.
> 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 <vi...@apache.org> 于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 <xu...@gmail.com>
> 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 vbalaji@apache.org <vb...@apache.org>
> > wrote:
> >
> > > +1 on standardizing code formatting. On Tuesday, August 18, 2020,
> > > 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com>
> > wrote:
> > >
> > > > Vote for mvn spotless:apply to do the auto fix.
> > > >
> > > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org>
> > > 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 <
> > xu.shiyan.raymond@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > in that case, yes, all for automation.
> > > > > >
> > > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <
> vinoth@apache.org>
> > > > > 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 <
> > > > xu.shiyan.raymond@gmail.com
> > > > > >
> > > > > > > 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
Posted by Vinoth Chandar <vi...@apache.org>.
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 <xu...@gmail.com>
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 vbalaji@apache.org <vb...@apache.org>
> wrote:
>
> > +1 on standardizing code formatting. On Tuesday, August 18, 2020,
> > 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com>
> wrote:
> >
> > > Vote for mvn spotless:apply to do the auto fix.
> > >
> > > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org>
> > 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 <
> xu.shiyan.raymond@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > in that case, yes, all for automation.
> > > > >
> > > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org>
> > > > 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 <
> > > xu.shiyan.raymond@gmail.com
> > > > >
> > > > > > 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
Posted by Shiyan Xu <xu...@gmail.com>.
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 vbalaji@apache.org <vb...@apache.org>
wrote:
> +1 on standardizing code formatting. On Tuesday, August 18, 2020,
> 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com> wrote:
>
> > Vote for mvn spotless:apply to do the auto fix.
> >
> > On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org>
> 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 <xu.shiyan.raymond@gmail.com
> >
> > > wrote:
> > >
> > > > in that case, yes, all for automation.
> > > >
> > > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org>
> > > 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 <
> > xu.shiyan.raymond@gmail.com
> > > >
> > > > > 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
Posted by "vbalaji@apache.org" <vb...@apache.org>.
+1 on standardizing code formatting. On Tuesday, August 18, 2020, 03:58:42 PM PDT, Vinoth Chandar <vi...@apache.org> 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 <ya...@gmail.com> wrote:
> Vote for mvn spotless:apply to do the auto fix.
>
> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org> 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 <xu...@gmail.com>
> > wrote:
> >
> > > in that case, yes, all for automation.
> > >
> > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org>
> > 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 <
> xu.shiyan.raymond@gmail.com
> > >
> > > > 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
Posted by Vinoth Chandar <vi...@apache.org>.
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 <ya...@gmail.com> wrote:
> Vote for mvn spotless:apply to do the auto fix.
>
> On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org> 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 <xu...@gmail.com>
> > wrote:
> >
> > > in that case, yes, all for automation.
> > >
> > > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org>
> > 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 <
> xu.shiyan.raymond@gmail.com
> > >
> > > > 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
Posted by Gary Li <ya...@gmail.com>.
Vote for mvn spotless:apply to do the auto fix.
On Thu, Aug 13, 2020 at 1:13 AM Vinoth Chandar <vi...@apache.org> 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 <xu...@gmail.com>
> wrote:
>
> > in that case, yes, all for automation.
> >
> > On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org>
> 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 <xu.shiyan.raymond@gmail.com
> >
> > > 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
Posted by Vinoth Chandar <vi...@apache.org>.
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 <xu...@gmail.com>
wrote:
> in that case, yes, all for automation.
>
> On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org> 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 <xu...@gmail.com>
> > 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
Posted by Shiyan Xu <xu...@gmail.com>.
in that case, yes, all for automation.
On Mon, Aug 10, 2020 at 7:12 PM Vinoth Chandar <vi...@apache.org> 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 <xu...@gmail.com>
> 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
Posted by Vinoth Chandar <vi...@apache.org>.
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 <xu...@gmail.com>
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
>