After bringing spotless plugin back to project, it would automatically fix
comment check error except for import order error, we need to fix this
error manually. In Apache Flink/Calcite, we also fix it manually, and will
also look for other plugins to fix import order error if exist.

Best,
Leesf

Vinoth Chandar <vin...@apache.org> 于2019年12月23日周一 下午4:55写道:

> I understand. I am saying - we should automate all of this formatting.. :)
>
> How do other projects do it? Other folks, who worked on the code
> refactoring/formatting, may be you can also chime in?
>
> On Mon, Dec 23, 2019 at 12:24 AM lamberken <lamber...@163.com> wrote:
>
> > Hi @Vinoth,
> >
> >
> > The ImportOrder is a custom rule, IDE may can not reformat codes rightly.
> > We can highlight this rule on contributing guide.
> >
> >
> > The new ImportOrder rule split import statements into groups and groups
> > are separated by one blank line.
> > These groups are 1) org.apache.hudi   2) third party imports   3) javax
> >  4) java   5) static
> >
> >
> > For example
> >
> >
> /---------------------------------------------------------------------------
> > package org.apache.hudi.metrics;
> >
> > import org.apache.hudi.config.HoodieWriteConfig;
> > import org.apache.hudi.exception.HoodieException;
> >
> > import com.google.common.base.Preconditions;
> > import org.apache.log4j.LogManager;
> > import org.apache.log4j.Logger;
> >
> > import javax.management.remote.JMXConnectorServer;
> > import javax.management.remote.JMXConnectorServerFactory;
> > import javax.management.remote.JMXServiceURL;
> >
> > import java.io.Closeable;
> > import java.lang.management.ManagementFactory;
> > import java.rmi.registry.LocateRegistry;
> >
> > public class JmxMetricsReporter extends MetricsReporter {
> >
> >
> >
> /---------------------------------------------------------------------------
> >
> >
> > best,
> > lamber-ken
> >
> >
> >
> >
> >
> > At 2019-12-23 15:45:27, "Vinoth Chandar" <vin...@apache.org> wrote:
> > >+1 on 1/3 and improving the contributing guide. But on 2,  IMO it would
> be
> > >overloading PULL_REQUEST_TEMPLATE.
> > >
> > >Bigger point here is: We need a fully automated way of formatting code
> > >either using IDE or using something like spotless.
> > >I pulled the checkstyle rules into intellij and even then I noticed that
> > it
> > >does not apply all rules while formatting (e.g line breaks between
> import
> > >groups).
> > >
> > >On Sun, Dec 22, 2019 at 11:36 PM lamberken <lamber...@163.com> wrote:
> > >
> > >> Hi Vinoth,
> > >>
> > >>
> > >> Here are some of my points:
> > >>
> > >>
> > >> 1, When developers are not familiar with checkstyle rules, they feel
> > >> uncomfortable. I think its a good idea to
> > >> make the instructions on contributing guide work with the checkstyle
> > rules
> > >> we already have.
> > >>
> > >>
> > >> 2, We can also prompt users in the PULL_REQUEST_TEMPLATE about how to
> > >> check code style by themself manually.
> > >>
> > >>
> > >> 3, The code of the current project has met the checkstyle rules, we
> just
> > >> need handle the incremental codes.
> > >>
> > >>
> > >> 4, here are some useful tools which can be placed on contributing
> guide.
> > >> 1) mvn scalastyle:check
> > >> 2) mvn checkstyle:check
> > >> 3) https://checkstyle.sourceforge.io/index.html
> > >> 4) http://www.scalastyle.org/rules-1.0.0.html
> > >>
> > >>
> > >> best,
> > >> lamber-ken
> > >> On 12/23/2019 13:03,Vinoth Chandar<vin...@apache.org> wrote:
> > >> Hello all,
> > >>
> > >> I know a bunch of work has happened to format the code base, closer to
> > what
> > >> other project are doing..
> > >>
> > >> While working through some checkstyle violations today, I noticed that
> > the
> > >> IDE formatting is now out of date with the checkstyle enforced?
> Manually
> > >> fixing these checkstyle issues are not a very productive use of time
> > IMHO.
> > >>
> > >> http://hudi.apache.org/contributing.html#ide-setup
> > >>
> > >> Can we make the instructions here work with the checkstyle rules we
> > already
> > >> have? Goal should be that formatting code in IntelliJ (or other IDEs)
> > >> should autofix it so that checkstyle passes..
> > >>
> > >> thoughts?
> > >>
> > >> Thanks
> > >> Vinoth
> > >>
> >
>

Reply via email to