My 2 cents:
I am also a big fan of code formatting in general, given that its
fully automated. But if that comes at the cost of taking some time off
everyone's time(manual fixing), we need to think through if its really
worth it. Especially, wrt import order, I am not sure if that really adds a
lot of value. Anyone who work with an IDE, all imports are collapsed and no
one gets to see that only. So, what ever order or grouping we follow, it
doesn't matter much. So, having said that, I am not up for spending 10 mins
everytime we create or update a PR for this import ordering rule. If I plan
to work on two PRs one followed by other, and if I spend 15 odd mins in
fixing first PR just for code formatting when creating one, I might lose
interest to continue working with 2nd. So, would prefer to avoid spending
time manually for code formatting in general.
On Mon, Dec 23, 2019 at 7:43 AM Vinoth Chandar <vin...@apache.org> wrote:
> Can we exhaustively list all that will be manually even after spotless
> plugin is brought back?
>
> On Mon, Dec 23, 2019 at 3:01 AM leesf <leesf0...@gmail.com> wrote:
>
> > 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
> > > > >>
> > > >
> > >
> >
>
--
Regards,
-Sivabalan
