+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 <[email protected]> wrote: > > totally +1 for spotless. > > >> On Thu, Aug 20, 2020 at 8:53 AM leesf <[email protected]> wrote: >> >> +1 on using mvn spotless:apply to fix the codestyle. >> >> Bhavani Sudha <[email protected]> 于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 <[email protected]> 于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 <[email protected]> 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
