We've seen the sync to github lag before when there are lots of changes, like when the docs were contributed. Hopefully the changes should show up there shortly!
-Dan On Fri, Oct 21, 2016 at 2:53 PM, Jared Stewart <jstew...@pivotal.io> wrote: > 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 > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>> > >>>>> > >>> > >>> > >> >
