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