Sounds like a plan! Let’s do this :-) Anthony
> 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 >>>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>> >>> >> >>
