FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the Apache git repo.
Is there a way to reformat a branch and then rebase on develop to minimize conflicts? -Kirk On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart <jstew...@pivotal.io> wrote: > 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 > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
