I haven’t dug in to the configuration of eclipseOrganizeImports, so for now spotless is not enforcing import ordering. If we verify that the import ordering specified by this file looks good, I can turn that on in the spotless config.
— Jared > On Oct 21, 2016, at 1:38 PM, Kirk Lund <kl...@pivotal.io> wrote: > > I just did a pull and now I'm reviewing the formatters under etc/. > > -rw-rw-r-- 1 klund staff 36004 Oct 21 13:36 eclipse-java-google-style.xml > -rw-rw-r-- 1 klund staff 110 Sep 19 11:56 > eclipseOrganizeImports.importorder > -rw-rw-r-- 1 klund staff 20653 Oct 21 13:36 > intellij-java-google-style.xml > > What's the status of organizing imports? And do we still use > eclipseOrganizeImports.importorder for imports in Eclipse? > > Thanks, > Kirk > > > On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl <asf.mbr...@gmail.com> wrote: > >> Thanks Jared for the suggestion of Spotless and follow-up work. >> >> This is now completed and checked into develop. As this does touch many >> files, be prepared the next time you pull. >> >> --Mark >> >> >> >> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart <jstew...@pivotal.io> >> wrote: >> >>> Done! :) >>> >>> - Jared >>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> wrote: >>>> >>>> One more time! :) >>>> >>>> Conflicting files >>>> geode-core/src/test/java/org/apache/geode/disttx/ >> PRDistTXDUnitTest.java >>>> geode-core/src/test/java/org/apache/geode/disttx/ >>> PRDistTXWithVersionsDUnitTest.java >>>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ >>> PRTransactionDUnitTest.java >>>> >>>> --Mark >>>> >>>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart <jstew...@pivotal.io> >>> wrote: >>>> >>>>> I just pulled and rebased onto develop, and force pushed into the >>> existing >>>>> pull request. It should be clean to merge in now. >>>>> >>>>> Thanks, >>>>> Jared >>>>>> On Oct 21, 2016, at 11:57 AM, Mark Bretl <asf.mbr...@gmail.com> >> wrote: >>>>>> >>>>>> I believe there is enough consensus here to check this into develop. >>>>>> >>>>>> Jared, due to recent checkins into develop, can you update the pull >>>>> request >>>>>> one more time? Trying to make this as clean as possible. I will check >>>>> into >>>>>> develop after the update, unless someone else gets to it first. >>>>>> >>>>>> All, can we hold checkins on develop until the new formatter is >>> applied? >>>>>> >>>>>> Thanks, >>>>>> >>>>>> --Mark >>>>>> >>>>>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe <kh...@pivotal.io> >>> wrote: >>>>>> >>>>>>> +1 >>>>>>> >>>>>>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt < >>> bschucha...@pivotal.io> >>>>>>> wrote: >>>>>>>> >>>>>>>> +1 >>>>>>>> >>>>>>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit : >>>>>>>>> +1 >>>>>>>>> >>>>>>>>> >>>>>>>>> On 20/10/16 4:56 pm, Mark Bretl wrote: >>>>>>>>>> +1 as well... >>>>>>>>>> >>>>>>>>>> - Pulled changes >>>>>>>>>> - Executed './gradlew clean build' and was successful. >>>>>>>>>> - Modified a couple of random files to test >>>>>>>>>> - Ran './gradlew clean build' again and failed expectedly >>>>>>>>>> - Ran './gradlew spotlessApply', task was successful >>>>>>>>>> - Ran './gradlew clean build' and succeeded >>>>>>>>>> >>>>>>>>>> Great addition! As long as others are good with the formatter, >>> then I >>>>>>> am >>>>>>>>>> good. >>>>>>>>>> >>>>>>>>>> --Mark >>>>>>>>>> >>>>>>>>>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund <kl...@apache.org> >>> wrote: >>>>>>>>>> >>>>>>>>>>> +1 I just added my approval to the PR (and again here) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart < >>> jstew...@pivotal.io >>>>>> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> I have opened a pull request here <https://github.com/apache/ >>>>>>>>>>>> incubator-geode/pull/268> to enable the Spotless plugin and to >>>>>>> switch to >>>>>>>>>>>> the Google Java Style formatter templates. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> On Oct 18, 2016, at 4:32 PM, Kirk Lund <kl...@pivotal.io> >>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> For reference TRAC #38741 was a bug with the summary >>> "EOFException >>>>>>>>>>> during >>>>>>>>>>>>> deserialize on client update with copy-on-read=true" >>>>>>>>>>>>> >>>>>>>>>>>>> -Kirk >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart < >>>>> jstew...@pivotal.io >>>>>>>> >>>>>>>>>>>> wrote: >>>>>>>>>>>>>> To give everyone an update, using the Google Java Style >> eclipse >>>>>>>>>>> template >>>>>>>>>>>>>> there is an issue spotlessCheck where fails for >>>>>>>>>>>>>> geode-core/src/test/java/org/apache/geode/cache30/ >>>>>>>>>>>> Bug38741DUnitTest.java >>>>>>>>>>>>>> even if you run it directly after spotlessApply. This needs >> to >>> be >>>>>>>>>>>>>> investigated and fixed before I can open a pull request to >>> enable >>>>>>>>>>>> spotless. >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Oct 14, 2016, at 4:57 PM, Dan Smith <dsm...@pivotal.io> >>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> +1 - The formatting looks better now. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> -Dan >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart < >>>>>>> jstew...@pivotal.io >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>> I agree that the formatter needs fixing up. Our wiki < >>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/GEODE/Code+ >>>>>>> Style+Guide> >>>>>>>>>>>>>> says >>>>>>>>>>>>>>>> that we follow the Google Java Style guide, but that is not >>>>>>> actually >>>>>>>>>>>>>> what’s >>>>>>>>>>>>>>>> in our formatter templates. I pushed a new branch < >>>>>>>>>>>> https://github.com/ >>>>>>>>>>>>>>>> jaredjstewart/incubator-geode/tree/ >>> spotlessPluginGoogleStyle> >>>>>>> that >>>>>>>>>>>>>> points >>>>>>>>>>>>>>>> spotless at the actual Google Java Style template, and I >>> think >>>>> it >>>>>>>>>>>> makes >>>>>>>>>>>>>>>> both of the examples you found look better. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Oct 13, 2016, at 10:11 AM, Dan Smith < >> dsm...@pivotal.io> >>>>>>> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> +1 for adding this to ./gradlew build >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> But I think we might want to fix up the formatter a bit >>> before >>>>>>>>>>>>>>>> reformatting >>>>>>>>>>>>>>>>> the code. I tried running spotlessApply, and it did some >>>>>>>>>>> unfortunate >>>>>>>>>>>>>>>>> reformatting of code to make it less readable. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> One problem is with method chaining. We have a few >> different >>>>>>>>>>> factory >>>>>>>>>>>>>> APIs >>>>>>>>>>>>>>>>> that encourage method chaining, and it put all the method >>>>> calls >>>>>>> on >>>>>>>>>>> a >>>>>>>>>>>>>>>> single >>>>>>>>>>>>>>>>> line. For example here's one change: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> - ClientCacheFactory ccf = new ClientCacheFactory() >>>>>>>>>>>>>>>>> - >>>>>>>>>>>>>>>>> .addPoolServer(NetworkUtils.getServerHostName(server1. >>>>>>> getHost()), >>>>>>>>>>>>>> port) >>>>>>>>>>>>>>>>> - .set(SECURITY_CLIENT_AUTH_INIT, >>>>>>>>>>>>>>>>> UserPasswordAuthInit.class.getName() + ".create") >>>>>>>>>>>>>>>>> - .set(SECURITY_PREFIX+"username", "root") >>>>>>>>>>>>>>>>> - .set(SECURITY_PREFIX+"password", "root"); >>>>>>>>>>>>>>>>> + ClientCacheFactory ccf = new >>>>>>>>>>>>>>>>> ClientCacheFactory().addPoolServer(NetworkUtils. >>>>>>>>>>>>>>>> getServerHostName(server1.getHost()), >>>>>>>>>>>>>>>>> port).set(SECURITY_CLIENT_AUTH_INIT, >>>>>>> UserPasswordAuthInit.class. >>>>>>>>>>>>>> getName() >>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>> ".create").set(SECURITY_PREFIX + "username", >>>>>>>>>>>>>> "root").set(SECURITY_PREFIX >>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>> "password", "root"); >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I see a similar problem where it put array initialization >>> all >>>>>>> on a >>>>>>>>>>>>>> single >>>>>>>>>>>>>>>>> line: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> + public void testMultiColOrderByWithIndexRe >>>>>>> sultWithProjection() >>>>>>>>>>>>>> throws >>>>>>>>>>>>>>>>> Exception { >>>>>>>>>>>>>>>>> String queries[] = { >>>>>>>>>>>>>>>>> // Test case No. IUMR021 >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID desc, pkid desc ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID asc, pkid asc ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 and ID < 20 order by ID asc, pkid asc ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 and ID < 20 order by ID desc , pkid desc", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID >= 10 and ID <= 20 order by ID desc, pkid asc ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID >= 10 and ID <= 20 order by ID asc, pkid desc", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID != 10 order by ID asc , pkid desc", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID != 10 order by ID desc, pkid asc ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID desc, pkid desc limit 5", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID asc, pkid asc limit 5", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 and ID < 20 order by ID asc, pkid desc >> limit 5 >>>>> ", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 and ID < 20 order by ID desc, pkid asc limit >>> 5", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID >= 10 and ID <= 20 order by ID desc, pkid desc >>> limit >>>>>>> 5", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID >= 10 and ID <= 20 order by ID asc, pkid asc >> limit >>>>> 5", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID != 10 order by ID asc , pkid desc limit 10", >>>>>>>>>>>>>>>>> - "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID != 10 order by ID desc, pkid desc limit 10", >>>>>>>>>>>>>>>>> - >>>>>>>>>>>>>>>>> - }; >>>>>>>>>>>>>>>>> + "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID desc, pkid desc ", "SELECT ID, >>>>>>>>>>>> description, >>>>>>>>>>>>>>>>> createTime, pkid FROM /portfolio1 pf1 where ID > 10 order >> by >>>>> ID >>>>>>>>>>> asc, >>>>>>>>>>>>>> pkid >>>>>>>>>>>>>>>>> asc ", "SELECT ID, description, createTime, pkid FROM >>>>>>> /portfolio1 >>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 and ID < 20 order by ID asc, pkid asc ", >>> "SELECT >>>>>>>>>>> ID, >>>>>>>>>>>>>>>>> description, createTime, pkid FROM /portfolio1 pf1 where >> ID >>>> >>>>> 10 >>>>>>>>>>> and >>>>>>>>>>>>>> ID < >>>>>>>>>>>>>>>>> 20 order by ID desc , pkid desc", "SELECT ID, >> description, >>>>>>>>>>>>>> createTime, >>>>>>>>>>>>>>>>> pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20 >> order >>> by >>>>>>> ID >>>>>>>>>>>> desc, >>>>>>>>>>>>>>>>> pkid asc ", "SELECT ID, description, createTime, pkid >> FROM >>>>>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>>>>> pf1 where ID >= 10 and ID <= 20 order by ID asc, pkid >> desc", >>>>>>>>>>> "SELECT >>>>>>>>>>>>>>>> ID, >>>>>>>>>>>>>>>>> description, createTime, pkid FROM /portfolio1 pf1 where >> ID >>> != >>>>>>> 10 >>>>>>>>>>>> order >>>>>>>>>>>>>>>> by >>>>>>>>>>>>>>>>> ID asc , pkid desc", "SELECT ID, description, >> createTime, >>>>> pkid >>>>>>>>>>> FROM >>>>>>>>>>>>>>>>> /portfolio1 pf1 where ID != 10 order by ID desc, pkid asc >> ", >>>>>>>>>>>>>>>>> + "SELECT ID, description, createTime, pkid FROM >>>>>>>>>>> /portfolio1 >>>>>>>>>>>>>> pf1 >>>>>>>>>>>>>>>>> where ID > 10 order by ID desc, pkid desc limit 5", >> "SELECT >>>>>>> ID, >>>>>>>>>>>>>>>>> description, createTime, pkid FROM /portfolio1 pf1 where >> ID >>>> >>>>> 10 >>>>>>>>>>>> order >>>>>>>>>>>>>> by >>>>>>>>>>>>>>>>> ID asc, pkid asc limit 5", "SELECT ID, description, >>>>>>> createTime, >>>>>>>>>>>> pkid >>>>>>>>>>>>>>>> FROM >>>>>>>>>>>>>>>>> /portfolio1 pf1 where ID > 10 and ID < 20 order by ID asc, >>>>> pkid >>>>>>>>>>> desc >>>>>>>>>>>>>>>> limit >>>>>>>>>>>>>>>>> 5 ", "SELECT ID, description, createTime, pkid FROM >>>>>>> /portfolio1 >>>>>>>>>>> pf1 >>>>>>>>>>>>>>>> where >>>>>>>>>>>>>>>>> ID > 10 and ID < 20 order by ID desc, pkid asc limit 5", >>>>> "SELECT >>>>>>>>>>>> ID, >>>>>>>>>>>>>>>>> description, createTime, pkid FROM /portfolio1 pf1 where >> ID >>>> = >>>>>>> 10 >>>>>>>>>>> and >>>>>>>>>>>>>> ID >>>>>>>>>>>>>>>> <= >>>>>>>>>>>>>>>>> 20 order by ID desc, pkid desc limit 5", "SELECT ID, >>>>>>> description, >>>>>>>>>>>>>>>>> createTime, pkid FROM /portfolio1 pf1 where ID >= 10 and >> ID >>> <= >>>>>>> 20 >>>>>>>>>>>> order >>>>>>>>>>>>>>>> by >>>>>>>>>>>>>>>>> ID asc, pkid asc limit 5", "SELECT ID, description, >>>>>>> createTime, >>>>>>>>>>>> pkid >>>>>>>>>>>>>>>> FROM >>>>>>>>>>>>>>>>> /portfolio1 pf1 where ID != 10 order by ID asc , pkid desc >>>>> limit >>>>>>>>>>> 10", >>>>>>>>>>>>>>>>> "SELECT ID, description, createTime, pkid FROM >> /portfolio1 >>>>> pf1 >>>>>>>>>>>> where >>>>>>>>>>>>>> ID >>>>>>>>>>>>>>>>> != 10 order by ID desc, pkid desc limit 10", >>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>> + }; >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart < >>>>>>>>>>> jstew...@pivotal.io> >>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>> The task is fully suppressible with -x spotlessCheck. >>> Also, >>>>> if >>>>>>>>>>> you >>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>>> any formatter errors you can automatically fix them with >>>>>>> 'gradle >>>>>>>>>>>>>>>>>> spotlessApply’. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> On Oct 13, 2016, at 9:40 AM, Kevin Duling < >>>>> kdul...@pivotal.io >>>>>>>> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>> If we made formatting a warning, then people would >>> probably >>>>>>>>>>> quickly >>>>>>>>>>>>>>>>>> ignore >>>>>>>>>>>>>>>>>>> it. >>>>>>>>>>>>>>>>>>> If we made formatting an error, we need to be sure we >>> don't >>>>>>> get >>>>>>>>>>> in >>>>>>>>>>>> to >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>> situation where <editor of choice>'s formatter is not in >>>>>>>>>>> agreement >>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>> build's checker. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> I can live with an additional 17 seconds as well. And >>>>> Jared's >>>>>>>>>>>>>> already >>>>>>>>>>>>>>>>>>> reduced the build time locally by 50%. But I still want >>> the >>>>>>>>>>>> ability >>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>> suppress the check similar to -x javadoc. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 9:58 PM, William Markito < >>>>>>>>>>>>>> wmark...@pivotal.io> >>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> This sounds really good to me as well. +1 >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart < >>>>>>>>>>>> jstew...@pivotal.io >>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> This is running locally on my laptop. Since Spotless >> is >>>>>>> only >>>>>>>>>>>> doing >>>>>>>>>>>>>>>>>> code >>>>>>>>>>>>>>>>>>>>> formatting and not any other static analysis, it >> already >>>>>>> has 0 >>>>>>>>>>>>>>>> errors. >>>>>>>>>>>>>>>>>>>>> (Other than, of course, formatting not consistent with >>> the >>>>>>>>>>>>>> template.) >>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016, at 4:11 PM, Kenneth Howe < >>>>>>> kh...@pivotal.io> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>> Agree with Mark, this has to work with 0 errors >> before >>> it >>>>>>>>>>> would >>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>> useful in precheckin. I think I could live with an >>>>>>> additional >>>>>>>>>>> 17 >>>>>>>>>>>>>>>>>> seconds >>>>>>>>>>>>>>>>>>>>> most of the time for running the spotlessCheck as >>>>> suggested. >>>>>>>>>>>>>>>>>>>>>> Jared, Is that 17 seconds running locally on your >>> laptop >>>>> or >>>>>>>>>>> on a >>>>>>>>>>>>>>>> more >>>>>>>>>>>>>>>>>>>>> capable machine? >>>>>>>>>>>>>>>>>>>>>> Ken >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016, at 3:39 PM, Jared Stewart < >>>>>>>>>>>> jstew...@pivotal.io> >>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>> If you want to try it out, I pushed a branch to my >>> Geode >>>>>>> repo >>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>>> contains this change: >>>>>>>>>>>>>>>>>>>>>>> https://github.com/jaredjstewart/incubator-geode/ >>>>>>>>>>>>>>>> tree/spotlessPlugin >>>>>>>>>>>>>>>>>>>> < >>>>>>>>>>>>>>>>>>>>> https://github.com/jaredjstewart/incubator-geode/ >>>>>>>>>>>>>> tree/spotlessPlugin >>>>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider < >>>>>>>>>>>>>>>>>> dschnei...@pivotal.io >>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>> I like Dan's idea of catching formatting issues >>> earlier >>>>>>> but >>>>>>>>>>> I >>>>>>>>>>>>>>>> think >>>>>>>>>>>>>>>>>>>>> adding >>>>>>>>>>>>>>>>>>>>>>>> 5-10 minutes to "build" would be too much. >> Currently >>>>> when >>>>>>>>>>> I'm >>>>>>>>>>>>>>>> trying >>>>>>>>>>>>>>>>>>>>> to do >>>>>>>>>>>>>>>>>>>>>>>> a quick build I use -xjavadoc. I'd probably do the >>> same >>>>>>> for >>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>> target if >>>>>>>>>>>>>>>>>>>>>>>> it was part of build until I'm ready to do a >>>>> precheckin. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Mark, wouldn't running the formatter on all our >> java >>>>>>> files >>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>> checking >>>>>>>>>>>>>>>>>>>>>>>> them in get these issues down to 0? >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer < >>>>>>>>>>>>>>>>>>>> ukohlme...@pivotal.io >>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> +1 - adding checkstyle to precheckin. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> If the developer uses the provided templates ( >>>>> eclipse + >>>>>>>>>>>>>>>> intellij) >>>>>>>>>>>>>>>>>>>>> then >>>>>>>>>>>>>>>>>>>>>>>>> most of the formatting issues should be handled >>> before >>>>>>>>>>>>>>>> precheckin. >>>>>>>>>>>>>>>>>>>>> Also, if >>>>>>>>>>>>>>>>>>>>>>>>> a developer has a questionable coding style, that >>>>> should >>>>>>>>>>>> lessen >>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>>>>>>>>> developer will have resolve the issues before >> being >>>>>>> able to >>>>>>>>>>>>>>>> commit. >>>>>>>>>>>>>>>>>>>>>>>>> I also believe that this should not be an >>> overbearing >>>>>>> and >>>>>>>>>>>>>>>> intrusive >>>>>>>>>>>>>>>>>>>>>>>>> process. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> --Udo >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On 13/10/16 6:36 am, Mark Bretl wrote: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Dan, >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> There is some extra amount of time, 5-10 minutes >>>>> extra >>>>>>> for >>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>> entire >>>>>>>>>>>>>>>>>>>>>>>>>> project (depending on your CPU). I think the real >>>>>>> issue to >>>>>>>>>>>>>>>> adding >>>>>>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>> precheckin target and have it be 'effective' is >> it >>>>>>> needs >>>>>>>>>>> to >>>>>>>>>>>>>> run >>>>>>>>>>>>>>>>>>>>>>>>>> successfully, otherwise it would turn into noise >>> most >>>>>>> of >>>>>>>>>>> the >>>>>>>>>>>>>>>> time >>>>>>>>>>>>>>>>>> I >>>>>>>>>>>>>>>>>>>>> think. >>>>>>>>>>>>>>>>>>>>>>>>>> We need to get the issues down to 0 or manage to >>> set >>>>> a >>>>>>> new >>>>>>>>>>>>>>>>>> baseline >>>>>>>>>>>>>>>>>>>>> (not >>>>>>>>>>>>>>>>>>>>>>>>>> the best idea), which is a lot of work, to make >> it >>>>> run >>>>>>>>>>>>>>>>>>>> successfully. >>>>>>>>>>>>>>>>>>>>> Right >>>>>>>>>>>>>>>>>>>>>>>>>> now, if you run the target, it will fail every >> time >>>>>>> since >>>>>>>>>>>>>> there >>>>>>>>>>>>>>>>>>>>>>>>>> outstanding >>>>>>>>>>>>>>>>>>>>>>>>>> issues in the code and very hard to tell what >>> issues >>>>>>> were >>>>>>>>>>>>>>>>>>>> introduced. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> --Mark >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith < >>>>>>>>>>>>>> dsm...@pivotal.io> >>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> Seems like it should run as part of the build >>> target. >>>>>>> The >>>>>>>>>>>> only >>>>>>>>>>>>>>>>>>>>> reason to >>>>>>>>>>>>>>>>>>>>>>>>>>> make it part of precheckin is if it takes a long >>>>> time, >>>>>>>>>>>>>>>> otherwise >>>>>>>>>>>>>>>>>>>>> people >>>>>>>>>>>>>>>>>>>>>>>>>>> should get fast feedback they need to change >> their >>>>>>> code >>>>>>>>>>>>>> before >>>>>>>>>>>>>>>>>>>> they >>>>>>>>>>>>>>>>>>>>> push. >>>>>>>>>>>>>>>>>>>>>>>>>>> -Dan >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart >> < >>>>>>>>>>>>>>>>>>>>> jstew...@pivotal.io> >>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> +1 to running during the precheckin target as >> well >>>>> as >>>>>>>>>>>> Travis >>>>>>>>>>>>>> CI >>>>>>>>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" < >>>>>>>>>>>>>>>>>>>>> dschnei...@pivotal.io> >>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> If Travis CI is only run on pull requests then >>> that >>>>>>> is >>>>>>>>>>> not >>>>>>>>>>>>>>>>>> enough >>>>>>>>>>>>>>>>>>>>>>>>>>>> because >>>>>>>>>>>>>>>>>>>>>>>>>>>> committers do not submit pull requests. Having >> it >>>>> run >>>>>>>>>>>> during >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>> gradle >>>>>>>>>>>>>>>>>>>>>>>>>>>>> build or precheckin target is also needed. In >>>>>>> addition >>>>>>>>>>> to >>>>>>>>>>>>>>>> that >>>>>>>>>>>>>>>>>> I >>>>>>>>>>>>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>>>>>>>>>>> wanted PRs to be checked. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 11:12 AM, Jared >> Stewart >>> < >>>>>>>>>>>>>>>>>>>>> jstew...@pivotal.io> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> It would certainly be necessary to make sure >>> that >>>>>>> the >>>>>>>>>>>> code >>>>>>>>>>>>>>>>>> style >>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>>>>>> enforced is sensible, e.g. doe not use wildcard >>>>>>> imports. >>>>>>>>>>>> We >>>>>>>>>>>>>>>>>>>> would >>>>>>>>>>>>>>>>>>>>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>>>>>>>>>> want to make one large commit to format all >>>>> existing >>>>>>>>>>> code >>>>>>>>>>>>>>>> before >>>>>>>>>>>>>>>>>>>>>>>>>>>>> turning >>>>>>>>>>>>>>>>>>>>>>>>>>>>> this on. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Mark - Thank you for the information about >> the >>>>>>>>>>> existing >>>>>>>>>>>>>>>> setup. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Mark, Darrel, Kevin - Given all of your >>>>> comments, I >>>>>>>>>>>> think >>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>> might >>>>>>>>>>>>>>>>>>>>>>>>>>>>> make >>>>>>>>>>>>>>>>>>>>>>>>>>>> more sense to add the flag to enable it in >> Travis >>>>> CI >>>>>>>>>>>> rather >>>>>>>>>>>>>>>> than >>>>>>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>>>>>>>>>>> part >>>>>>>>>>>>>>>>>>>>>>>>>>>>> of the build. This way your build pass >>>>> regardless >>>>>>> of >>>>>>>>>>>>>>>>>>>> whitespace, >>>>>>>>>>>>>>>>>>>>>>>>>>>>> but >>>>>>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> CI job would fail on PRs if they did not >> adhere >>>>> to >>>>>>> the >>>>>>>>>>>>>>>>>> standard >>>>>>>>>>>>>>>>>>>>>>>>>>>>> formatting. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Anthony - It doesn’t seem to me that turning >>> this >>>>>>> on >>>>>>>>>>>> would >>>>>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>> effect >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> of combining reformatting commits and logic >>>>>>> changes. >>>>>>>>>>>>>>>> Rather, >>>>>>>>>>>>>>>>>>>>> since >>>>>>>>>>>>>>>>>>>>>>>>>>>>> all >>>>>>>>>>>>>>>>>>>>>>>>>>>> code would already be formatted, there would no >>>>>>> longer >>>>>>>>>>> be >>>>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>>>>>>>>>> reformatting >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> commits except for single large commits when >>> the >>>>>>> code >>>>>>>>>>>>>> style >>>>>>>>>>>>>>>>>>>> file >>>>>>>>>>>>>>>>>>>>> was >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> updated. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016, at 11:01 AM, Bruce >> Schuchardt >>> < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bschucha...@pivotal.io >>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I like the idea of doing this but I don't >>> think >>>>>>>>>>>>>> Checkstyle >>>>>>>>>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>>>>>> enabled until all of the code is reformatted. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Also, last time I checked there was still a >>>>>>> problem >>>>>>>>>>>> with >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> IntelliJ >>>>>>>>>>>>>>>>>>>>>>>>>>>> auto-format settings. It still used wildcard >>>>>>> imports, >>>>>>>>>>>>>> which I >>>>>>>>>>>>>>>>>>>>>>>>>>>>> believe >>>>>>>>>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> don't allow. I've manually changed my >> settings >>> in >>>>>>>>>>>>>>>> Editor->Code >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Style->Java >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to "Use single class import" to correct that >>>>>>>>>>> problem. I >>>>>>>>>>>>>>>>>>>>> couldn't see >>>>>>>>>>>>>>>>>>>>>>>>>>>>> how >>>>>>>>>>>>>>>>>>>>>>>>>>>>> to get Gradle to do it. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Le 10/12/2016 à 10:28 AM, Anthony Baker a >>> écrit >>>>> : >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Source code with a consistent look-and-feel >>>>>>> makes it >>>>>>>>>>>>>>>> easier >>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> people >>>>>>>>>>>>>>>>>>>>>>>>>>>>> to join the project community and contribute. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Let’s continue to keep reformatting commits >>>>>>> separate >>>>>>>>>>>> from >>>>>>>>>>>>>>>>>>>> logic >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> changes—otherwise it’s too hard to review. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Anthony >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016, at 10:06 AM, Dan Smith < >>>>>>>>>>>>>>>> dsm...@pivotal.io> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> +1 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This might be a good time to reformat the >>> code >>>>>>>>>>> since >>>>>>>>>>>> I >>>>>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> think >>>>>>>>>>>>>>>>>>>>>>>>>>>> there >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are too many long lived feature branches >>>>>>> outstanding. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Dan >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 10:00 AM, Jared >>>>> Stewart >>>>>>> < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> jstew...@pivotal.io >>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I would like to advocate for adding a >>>>> Checkstyle < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://checkstyle >>>>>>>>>>>>>>>>>>>>>>>>>>>>> . >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sourceforge.net/> or Spotless < >>>>>>>>>>>>>> https://github.com/diffplug/ >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> spotless >>>>>>>>>>>>>>>>>>>>>>>>>>>>> gradle task to our build process to ensure >> that >>>>> all >>>>>>>>>>> code >>>>>>>>>>>>>>>>>> checked >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>>>>>>>> meets >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the formatting standards described on the >>> wiki < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/ >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> confluence/display/GEODE/Code+Style+Guide> >>> (and >>>>>>> in >>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> intellij/eclipse >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> formatter xml files in our repository). This >>>>> will >>>>>>>>>>>>>> alleviate >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> difficulties >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reviewing code when whitespace or formatting >>> has >>>>>>>>>>>> changed >>>>>>>>>>>>>>>>>> since >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all >>>>>>>>>>>>>>>>>>>>>>>>>>>> code >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> checked in will already comply with >> standards. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> ~/William >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>> >>> >>
