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 > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >> > >> > >
