I think you want this in place before bloom filters are released. Since it's the newest code, it is most at risk of receiving fixes and improvements. You're not going to want to use spotless after the feature is introduced and make back ports more difficult.
On Wed, Jan 22, 2020, 10:02 AM Driesprong, Fokko <[email protected]> wrote: > 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 > > > > > >
