I've rebased the PR: https://github.com/apache/parquet-mr/pull/730

I did some searching and as far as I can tell, spotless does not allow to
only apply it to VCS changed lines. If the forked repo also applies
spotless, then it would be possible to do a diff.

For me, I'm still interested in applying this, so we can keep our code
clean and consistent. For example, I would like to enforce the use of
braces, as it makes the code much more readable in my opinion.

Cheers, Fokko



Op do 9 jan. 2020 om 10:23 schreef Gabor Szadovszky <[email protected]>:

> Personally, I don't like formatting the whole code during minor version
> development. These changes make really hard cherry-picking changes to
> forked repos. It also makes hard to blame the code.
> It is great to have a common code style and formatting configuration but I
> would only apply them to the new lines. Let's do such changes that impacts
> the whole code base at the beginning of a new major version development
> where compatibility will break anyway.
>
> I am hesitating to give a -1, though. If everyone agrees on this is a good
> idea, I'm fine with that. So, let me give a -0.
>
> Cheers,
> Gabor
>
> On Wed, Jan 8, 2020 at 7:36 PM Ryan Blue <[email protected]>
> wrote:
>
> > +1 for spotless checks.
> >
> > On Wed, Jan 8, 2020 at 7:13 AM Driesprong, Fokko <[email protected]>
> > wrote:
> >
> > > Y'all,
> > >
> > > Recently Chen Junjie brought up the removal of trailing spaces within
> the
> > > code and the headers:
> > > https://github.com/apache/parquet-mr/pull/727#issuecomment-571562392
> > >
> > > I've been looking into this and looked into if we can apply something
> > like
> > > checkstyle to let it fail on trailing whitespace. However, it comes up
> > with
> > > a LOT of warnings on improper formatting, short variables, wrong import
> > > orders, etc.
> > > For Apache Avro we've added Spotless as a maven plugin:
> > > https://github.com/diffplug/spotless. Unlike checkstyle, spotless will
> > > also
> > > fix the formatting. Would this be something that others find useful?
> > > The main problem is that we need to apply this to the codebase, and
> this
> > > will break a lot of PR's, and it will mess up a bit of the version
> > control,
> > > because a lot of lines will be changed:
> > > https://github.com/apache/parquet-mr/pull/730/
> > >
> > > WDYT?
> > >
> > > Cheers, Fokko
> > >
> >
> >
> > --
> > Ryan Blue
> > Software Engineer
> > Netflix
> >
>

Reply via email to