+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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
