+1
> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <[email protected]> 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 <[email protected]> wrote:
>>>
>>>> +1 I just added my approval to the PR (and again here)
>>>>
>>>>
>>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <[email protected]>
>>>> 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 <[email protected]> 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 <[email protected]>
>>>>> 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 <[email protected]> wrote:
>>>>>>>>
>>>>>>>> +1 - The formatting looks better now.
>>>>>>>>
>>>>>>>> -Dan
>>>>>>>>
>>>>>>>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <[email protected]
>>>>>>> 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 <[email protected]> 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 testMultiColOrderByWithIndexResultWithProjection()
>>>>>>> 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 <
>>>> [email protected]>
>>>>>>>>> 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 <[email protected]>
>>>>>>> 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 <
>>>>>>> [email protected]>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> This sounds really good to me as well. +1
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart <
>>>>> [email protected]
>>>>>>>>>>>>> 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 <[email protected]>
>>>>>>> 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 <
>>>>> [email protected]>
>>>>>>>>>>>>> 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 <
>>>>>>>>>>> [email protected]
>>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>>>>>> 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 <
>>>>>>> [email protected]>
>>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>>> [email protected]>
>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> +1 to running during the precheckin target as well as
>>>>> Travis
>>>>>>> CI
>>>>>>>>>>>>>>>>>>>>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
>>>>>>>>>>>>>> [email protected]>
>>>>>>>>>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>>> [email protected]>
>>>>>>>>>>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>>>>>>>>>>> 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 <
>>>>>>>>> [email protected]>
>>>>>>>>>>>>>>>>>>>>>>>>> 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 <
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> [email protected]
>>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>
>
