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