By the way, when I pull develop from GitHub I don’t see these changes. Nor do I see them when I look here <https://github.com/apache/incubator-geode/commits/develop>. It seems that they were added to the apache git repo but not the apache GitHub repo, maybe due to the GitHub DNS issues going on today? Anyways just wanted to see if anyone else has a better explanation.
—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 >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>> >>> >>
