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