Fantastic, thanks for merging this in Mark. For anyone with outstanding work on branches made before this change, your life may be made easier by cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added Spotless) into your branch and then running ‘gradlew spotlessApply’ on it before attempting to merge into develop.
— Jared > On 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 >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>> >>>> >> >>
