You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Rui Wang <ru...@google.com.INVALID> on 2019/09/16 19:09:00 UTC
auto-format or fix checkstyle by maven command
Hi community,
Is there a maven command or auto-formatter to fix checkstyle error when
developing in Calcite repo? I am less familiar with maven and did some
searches in both [1] and Google but found no luck.
[1]: https://calcite.apache.org/develop/
-Rui
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <am...@apache.org>.
The minimum spotless maven can do is:
remove unused import
correct import order
check license header
To use it is also easy after enabling it in maven pom:
mvn spotless:check // check the style
mvn spotless:apply // apply spotless fix
The difference though is the default setting of code style of spotless
differs from maven checkstyle (for example: import ordering is difference).
Spotless's style can be also configurable by a file so seems it's feasible
to match maven checkstyle.
-Rui
On Mon, Sep 16, 2019 at 3:10 PM Rui Wang <am...@apache.org> wrote:
> Thanks. I will try to setup IDE then.
>
> I don't have a clear idea how to use spotless for maven. I could spend
> some time to explore it and if it's easy to setup, I will report it back to
> you.
>
>
> -Rui
>
> On Mon, Sep 16, 2019 at 12:51 PM Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
>
>> Rui>Import order and length of a single line of code is too long were
>> what I
>> faced.
>>
>> I guess both of them could be configured in IDE.
>> Even though it might look boring/complicated, configuring import order in
>> IDE pays off quickly.
>>
>> Rui>In Apache Beam, we are using gradle and have adopted diffplug/spotless
>>
>> It looks like there's spotless-maven-plugin, so we might move certain
>> checkstyle rules to spotless even before we migrate to Gradle.
>> Do you have cycles to implement relevant Spotless configuration?
>>
>> Vladimir
>>
>
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <am...@apache.org>.
Thanks. I will try to setup IDE then.
I don't have a clear idea how to use spotless for maven. I could spend some
time to explore it and if it's easy to setup, I will report it back to you.
-Rui
On Mon, Sep 16, 2019 at 12:51 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Rui>Import order and length of a single line of code is too long were what
> I
> faced.
>
> I guess both of them could be configured in IDE.
> Even though it might look boring/complicated, configuring import order in
> IDE pays off quickly.
>
> Rui>In Apache Beam, we are using gradle and have adopted diffplug/spotless
>
> It looks like there's spotless-maven-plugin, so we might move certain
> checkstyle rules to spotless even before we migrate to Gradle.
> Do you have cycles to implement relevant Spotless configuration?
>
> Vladimir
>
Re: auto-format or fix checkstyle by maven command
Posted by Vladimir Sitnikov <si...@gmail.com>.
Rui>Import order and length of a single line of code is too long were what I
faced.
I guess both of them could be configured in IDE.
Even though it might look boring/complicated, configuring import order in
IDE pays off quickly.
Rui>In Apache Beam, we are using gradle and have adopted diffplug/spotless
It looks like there's spotless-maven-plugin, so we might move certain
checkstyle rules to spotless even before we migrate to Gradle.
Do you have cycles to implement relevant Spotless configuration?
Vladimir
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <am...@apache.org>.
Import order and length of a single line of code is too long were what I
faced.
In Apache Beam, we are using gradle and have adopted
https://github.com/diffplug/spotless. So there is a command to fix unused
import, import ordering, line length, etc there (gradle spotlessApply).
-Rui
On Mon, Sep 16, 2019 at 12:27 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> Rui>when developing in Calcite repo?
>
> Hi, could you please clarify which issues do you run into the most?
>
> Rui>auto-formatter to fix checkstyle error
>
> That would really be awesome.
> Unfortunately, my experience with Checkstyle project 5 years ago was that
> core developers did not value "automatic fix of the issues".
>
> See example: https://github.com/checkstyle/checkstyle/pull/164
> I used to run into "import order is wrong" a lot, and Checkstyle is not
> very helpful with explaining on what should be done.
>
> However, then I configured import order, and now it generates the proper
> import order by default.
> It would be great if editorconfig implements
> https://github.com/editorconfig/editorconfig/issues/231 so IDEs could take
> configuration from there.
>
>
> Recently I've discovered https://github.com/diffplug/spotless , and I like
> how it prints sensible messages, and it allows to apply lots of fixes
> automatically.
> See example:
> https://travis-ci.org/vlsi/vlsi-release-plugins/jobs/584025923#L410
> We might want to migrate certain checks to spotless so the violations
> become fixable automatically.
>
> Vladimir
>
Re: auto-format or fix checkstyle by maven command
Posted by Julian Hyde <jh...@apache.org>.
I’m sorry that we do not have the technology to auto-correct. People also complain that there are coding standards that are not captured in check style rules, so we are caught between a rock and a hard place.
I agree, something like https://github.com/editorconfig/editorconfig/issues/231 <https://github.com/editorconfig/editorconfig/issues/231>, so that the editor is configured based on the project definition (pom.xml) would be nice.
Never underestimate the power of “git diff”. Check to see whether your editor has made changes that are not helpful, and back them out before submitting the PR. Then change your editor’s config so that next time it either the right thing or does nothing.
Julian
> On Sep 16, 2019, at 12:26 PM, Vladimir Sitnikov <si...@gmail.com> wrote:
>
> Rui>when developing in Calcite repo?
>
> Hi, could you please clarify which issues do you run into the most?
>
> Rui>auto-formatter to fix checkstyle error
>
> That would really be awesome.
> Unfortunately, my experience with Checkstyle project 5 years ago was that
> core developers did not value "automatic fix of the issues".
>
> See example: https://github.com/checkstyle/checkstyle/pull/164
> I used to run into "import order is wrong" a lot, and Checkstyle is not
> very helpful with explaining on what should be done.
>
> However, then I configured import order, and now it generates the proper
> import order by default.
> It would be great if editorconfig implements
> https://github.com/editorconfig/editorconfig/issues/231 so IDEs could take
> configuration from there.
>
>
> Recently I've discovered https://github.com/diffplug/spotless , and I like
> how it prints sensible messages, and it allows to apply lots of fixes
> automatically.
> See example:
> https://travis-ci.org/vlsi/vlsi-release-plugins/jobs/584025923#L410
> We might want to migrate certain checks to spotless so the violations
> become fixable automatically.
>
> Vladimir
Re: auto-format or fix checkstyle by maven command
Posted by Vladimir Sitnikov <si...@gmail.com>.
Rui>when developing in Calcite repo?
Hi, could you please clarify which issues do you run into the most?
Rui>auto-formatter to fix checkstyle error
That would really be awesome.
Unfortunately, my experience with Checkstyle project 5 years ago was that
core developers did not value "automatic fix of the issues".
See example: https://github.com/checkstyle/checkstyle/pull/164
I used to run into "import order is wrong" a lot, and Checkstyle is not
very helpful with explaining on what should be done.
However, then I configured import order, and now it generates the proper
import order by default.
It would be great if editorconfig implements
https://github.com/editorconfig/editorconfig/issues/231 so IDEs could take
configuration from there.
Recently I've discovered https://github.com/diffplug/spotless , and I like
how it prints sensible messages, and it allows to apply lots of fixes
automatically.
See example:
https://travis-ci.org/vlsi/vlsi-release-plugins/jobs/584025923#L410
We might want to migrate certain checks to spotless so the violations
become fixable automatically.
Vladimir
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <am...@apache.org>.
Ah. The trick is that there is a whitespace after "static", so it is
"static ".
-Rui
On Thu, Sep 26, 2019 at 2:40 PM Rui Wang <ru...@google.com> wrote:
> That's interesting. I explicitly added "static" to the end but spotless
> maven did not correct the order.
>
>
> -Rui
>
> On Thu, Sep 26, 2019 at 1:52 PM Vladimir Sitnikov <
> sitnikov.vladimir@gmail.com> wrote:
>
>> >One diff, for example,
>> >is that spotless imports static first but checkstyle prefers static last,
>> >which I haven't found a way to config.
>>
>> I agree Spotless documentation is a bit vague, however Spotless-Gradle
>> orders statics for me.
>>
>> Here's an example:
>> // import statics first
>> importOrder("static ", "java.", "javax", "org", "net", "com", "")
>> // import statics last
>> importOrder("java.", "javax", "org", "net", "com", "", "static ")
>>
>> The sample is taken from Apache JMeter:
>>
>> https://github.com/apache/jmeter/blob/6d4610d5e3c5c3128100df3189c3e2f4d7fbe8be/build.gradle.kts#L292
>>
>>
>> Vladimir
>>
>
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <ru...@google.com.INVALID>.
That's interesting. I explicitly added "static" to the end but spotless
maven did not correct the order.
-Rui
On Thu, Sep 26, 2019 at 1:52 PM Vladimir Sitnikov <
sitnikov.vladimir@gmail.com> wrote:
> >One diff, for example,
> >is that spotless imports static first but checkstyle prefers static last,
> >which I haven't found a way to config.
>
> I agree Spotless documentation is a bit vague, however Spotless-Gradle
> orders statics for me.
>
> Here's an example:
> // import statics first
> importOrder("static ", "java.", "javax", "org", "net", "com", "")
> // import statics last
> importOrder("java.", "javax", "org", "net", "com", "", "static ")
>
> The sample is taken from Apache JMeter:
>
> https://github.com/apache/jmeter/blob/6d4610d5e3c5c3128100df3189c3e2f4d7fbe8be/build.gradle.kts#L292
>
>
> Vladimir
>
Re: auto-format or fix checkstyle by maven command
Posted by Vladimir Sitnikov <si...@gmail.com>.
>One diff, for example,
>is that spotless imports static first but checkstyle prefers static last,
>which I haven't found a way to config.
I agree Spotless documentation is a bit vague, however Spotless-Gradle
orders statics for me.
Here's an example:
// import statics first
importOrder("static ", "java.", "javax", "org", "net", "com", "")
// import statics last
importOrder("java.", "javax", "org", "net", "com", "", "static ")
The sample is taken from Apache JMeter:
https://github.com/apache/jmeter/blob/6d4610d5e3c5c3128100df3189c3e2f4d7fbe8be/build.gradle.kts#L292
Vladimir
Re: auto-format or fix checkstyle by maven command
Posted by Rui Wang <ru...@google.com.INVALID>.
Thanks Danny.
To update spotless a bit: I played with the configuration of spotless on
import order to make it match with checkstyle. I found a few diff between
my customized spotless config and checkstyle (even though my config is
copied from checkstyle without minor modification). One diff, for example,
is that spotless imports static first but checkstyle prefers static last,
which I haven't found a way to config.
-Rui
On Mon, Sep 16, 2019 at 8:53 PM Danny Chan <yu...@gmail.com> wrote:
> If you use IntelliJ IDEA for developing, you can generate a check style
> conf xml file for Calcite, then use this conf file in your code style
> config for Java. You should first download a check style plugin.
>
> Best,
> Danny Chan
> 在 2019年9月17日 +0800 AM3:09，Rui Wang <ru...@google.com.invalid>，写道：
> > Hi community,
> >
> > Is there a maven command or auto-formatter to fix checkstyle error when
> > developing in Calcite repo? I am less familiar with maven and did some
> > searches in both [1] and Google but found no luck.
> >
> >
> > [1]: https://calcite.apache.org/develop/
> >
> > -Rui
>
Re: auto-format or fix checkstyle by maven command
Posted by Danny Chan <yu...@gmail.com>.
If you use IntelliJ IDEA for developing, you can generate a check style conf xml file for Calcite, then use this conf file in your code style config for Java. You should first download a check style plugin.
Best,
Danny Chan
在 2019年9月17日 +0800 AM3:09，Rui Wang <ru...@google.com.invalid>，写道：
> Hi community,
>
> Is there a maven command or auto-formatter to fix checkstyle error when
> developing in Calcite repo? I am less familiar with maven and did some
> searches in both [1] and Google but found no luck.
>
>
> [1]: https://calcite.apache.org/develop/
>
> -Rui