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