[GitHub] [geode] jinmeiliao commented on issue #2465: GEODE-5606: remove CategoryWithParameterizedRunnerFactory

2018-09-13 Thread GitHub
@kirklund you are right, the ticket does not consider other categories other than integration/distributed tests. Will close this. [ Full content available at: https://github.com/apache/geode/pull/2465 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout opened pull request #2467: GEODE-5714: Retry ssh connections in concourse

2018-09-13 Thread GitHub
Adding ConnectionAttempts=60 to all concourse scripts that are using ssh. We are seeing jobs occasionally fail with connection refused. If this is a transient issue, this change might fix it. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contrib

[GitHub] [geode] jinmeiliao closed pull request #2465: GEODE-5606: remove CategoryWithParameterizedRunnerFactory

2018-09-13 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2465 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-14 Thread GitHub
I ran the clang-format tool in CLion, apparently it didn't do everything "our" way, go figure. [ Full content available at: https://github.com/apache/geode-native/pull/352 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-14 Thread GitHub
If the file has a majority in one direction the formatting rule puts them all to the majority direction. When starting a new file you should prefer right. I suggest updating your formatting rules in CLion to auto format right and then clang format will keep the file in line. [ Full content avai

[GitHub] [geode-native] pdxcodemonkey commented on pull request #351: GEODE-5626: Fix crash in register all keys

2018-09-14 Thread GitHub
Renamed all instances `poolFactory` [ Full content available at: https://github.com/apache/geode-native/pull/351 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #2468: GEODE-3: remove dependency on sun.tools.java.CompilerError

2018-09-14 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has y

[GitHub] [geode] jinmeiliao opened pull request #2469: GEODE-3: add lib jars that will be missing in jdk9 and later

2018-09-14 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has y

[GitHub] [geode]

2018-09-14 Thread GitHub
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp opened pull request #2470: GEODE-3720: adding more error conditions

2018-09-14 Thread GitHub
- Adding a pattern to match which is the bugfix - Duplication reduction for readability Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a J

[GitHub] [geode] mhansonp commented on issue #2470: GEODE-3720: adding more error conditions

2018-09-14 Thread GitHub
@WireBaron @balesh2 @galen-pivotal @upthewaterspout @pdxrunner @kirklund Could you review this bug fix? [ Full content available at: https://github.com/apache/geode/pull/2470 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #2471: GEODE-3: do not use Assert in production code. use explicit exception.

2018-09-14 Thread GitHub
* turning on assertion in classloader would not play nicely in jdk9 and later. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticke

[GitHub] [geode] jinmeiliao opened pull request #2472: GEODE-3: not use our own SystemUtils to find java version

2018-09-14 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has y

[GitHub] [geode] mhansonp commented on pull request #2470: GEODE-3720: adding more error conditions

2018-09-14 Thread GitHub
Change here for bugfix [ Full content available at: https://github.com/apache/geode/pull/2470 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp commented on pull request #2470: GEODE-3720: adding more error conditions

2018-09-14 Thread GitHub
Change here for bugfix [ Full content available at: https://github.com/apache/geode/pull/2470 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jdeppe-pivotal commented on pull request #2469: GEODE-3: add lib jars that will be missing in jdk9 and later

2018-09-14 Thread GitHub
Please add alphabetically [ Full content available at: https://github.com/apache/geode/pull/2469 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jdeppe-pivotal commented on pull request #2469: GEODE-3: add lib jars that will be missing in jdk9 and later

2018-09-14 Thread GitHub
Please keep these in alphabetical order. [ Full content available at: https://github.com/apache/geode/pull/2469 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2468: GEODE-3: remove dependency on sun.tools.java.CompilerError

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2468 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2471: GEODE-3: do not use Assert in production code. use explicit exception.

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2471 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2472: GEODE-3: not use our own SystemUtils to find java version

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2472 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
would be good to explain the reason of switching the order of these two lines here, because I am wondering too. [ Full content available at: https://github.com/apache/geode/pull/2426 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
I think the old code is trying to turn all exception type into GemfireRestException with different messages. If we delete this block, we lose that purpose. [ Full content available at: https://github.com/apache/geode/pull/2426 ] This message was relayed via gitbox.apache.org for notifications@g

[GitHub] [geode] jinmeiliao commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
why do we have to make a distinction for GemfireRestException here? Would be nice to put a comment here explaining this. [ Full content available at: https://github.com/apache/geode/pull/2426 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2469: GEODE-3: add lib jars that will be missing in jdk9 and later

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2469 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp commented on pull request #2463: Geode 5428: Refactored DurableClientTests due to massive duplication

2018-09-14 Thread GitHub
There seems to be some delay with the registering of interest. I'm not fond of this wait, but there isn't a better way I saw. Nothing to check for that would provide good feedback. I am open to suggestions. [ Full content available at: https://github.com/apache/geode/pull/2463 ] This message was

[GitHub] [geode] jdeppe-pivotal commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
This goes together with `convertErrorAsJson`. The other exceptions, in this block, are very specific and the wrapping `GemFireRestException` can thus be specific and meaningful. (Bear in mind that the exception will ultimately be turned into json by `convertErrorAsJson` and returned to the user)

[GitHub] [geode] mhansonp commented on pull request #2463: Geode 5428: Refactored DurableClientTests due to massive duplication

2018-09-14 Thread GitHub
This is a magic timeout. Remove it and things fail. Add it in and things are happy. I have no idea what this is really affecting, but it is necessary to work properly. [ Full content available at: https://github.com/apache/geode/pull/2463 ] This message was relayed via gitbox.apache.org for not

[GitHub] [geode] jdeppe-pivotal commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
If we left this catch block then I think the exception should simply be returned as: ``` throw new GemfireRestException(e.getMessage()); ``` [ Full content available at: https://github.com/apache/geode/pull/2426 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jdeppe-pivotal commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
If we left this catch block then I think the exception should simply be returned as: ``` throw new GemfireRestException(e.getMessage()); ``` and that would also obviate the need for special handling in `convertErrorAsJson`. [ Full content available at: https://github.com/apache/geode/pull/2426

[GitHub] [geode] jinmeiliao opened pull request #2473: GEODE-3: fix analyzeDataSerializable test for java 9 and later

2018-09-14 Thread GitHub
this code change is covered by analyzeDataSerializable test already. this should not impact runs on jdk8, but the test was failing using jdk9 and later without this fix. Port the fix to develop to make sure this does not interfere with jdk8. Thank you for submitting a contribution to Apache Geo

[GitHub] [geode] PurelyApplied opened pull request #2474: GEODE-5600: Unify SCM property caching.

2018-09-14 Thread GitHub
* Refactor gradle tasks generating .buildinfo and GemFireVerion.properties files * Treat .buildinfo as the source of truth for those properties shared in GemFireVersion.properties Co-authored-by: Patrick Rhomberg Thank you for submitting a contribution to Apache Geode. In order to streamline th

[GitHub] [geode] gesterzhou closed pull request #2458: GEODE-5729: when DistributedCacheOperation needs 2 messages, should let

2018-09-14 Thread GitHub
[ pull request closed by gesterzhou ] [ Full content available at: https://github.com/apache/geode/pull/2458 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett closed pull request #2451: GEODE-5722 - Improve concourse deployment experience for releases

2018-09-14 Thread GitHub
[ pull request closed by pivotal-jbarrett ] [ Full content available at: https://github.com/apache/geode/pull/2451 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2422: GEODE-5688 Create task to generate all sources for better IDE behavior

2018-09-14 Thread GitHub
Is this afterEvaluate really necessary too? [ Full content available at: https://github.com/apache/geode/pull/2422 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2473: GEODE-3: Use the correct DataSerializer for TimeUnit in java 9 and later

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2473 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #2426: GEODE-5696: Refactor RestQueryAndFunctionIntegrationTest

2018-09-14 Thread GitHub
how about: throw new GemfireRestException(e.getMessage(), e); [ Full content available at: https://github.com/apache/geode/pull/2426 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #2475: GEODE-3: exclude internal packages when generating javadoc

2018-09-14 Thread GitHub
* javadoc task will fail if we include internal packages using java9 and later Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticke

[GitHub] [geode] jinmeiliao opened pull request #2476: GEODE-3: small java 9 compatibility fixes

2018-09-14 Thread GitHub
* upgrade library to be java9 compatible to fix running tomcat7sessionJUnitTest in java9 * fix InstallerJUnitTest * fix DeprecatedAgentLauncherIntegrationTest (only include -d64 in ProcessWrapper for Solaris machines) * PowerMock needs to ignore more modules in PulseControllerJunitTest in java9

[GitHub] [geode] jinmeiliao opened pull request #2477: GEODE-3: Do not use @PostConstruct annotation which is not supported …

2018-09-14 Thread GitHub
…in java9 * this fixes RestSecurityIntegrationTest Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR?

[GitHub] [geode] jinmeiliao opened pull request #2478: GEODE-3: remove dependency on management-agent.jar which is not inclu…

2018-09-14 Thread GitHub
…ded in java9 and later * this fix *launcher*IntegrationTests when running with java9 Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JI

[GitHub] [geode] kirklund commented on pull request #2476: GEODE-3: small java 9 compatibility fixes

2018-09-14 Thread GitHub
Does this mean that "-d64" is no longer a valid option for Mac or Linux? [ Full content available at: https://github.com/apache/geode/pull/2476 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #2479: GEODE-3: more memory usage beans are available in java9. GemFireStatS…

2018-09-14 Thread GitHub
…amplerIntegrationTest needs to use production code to find the right Statistics to use. * this fixes testLocalStatListenerRegistration test Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps ha

[GitHub] [geode] jinmeiliao opened pull request #2480: GEODE-4052: If MBeanProcessController determines a process is not ava…

2018-09-14 Thread GitHub
…ilable, skip using the FileProcessController * this would fix the issue that the StatusServerExitCodeAcceptanceTest.statusWithWrongPid taking too long to exit. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure th

[GitHub] [geode] agingade opened pull request #2481: GEODE-5605: After removeAll/PutAll messages are processed on replicas, check for cache close is done.

2018-09-14 Thread GitHub
An in-flight operation may not be seen by the new GII requester if the GII provider cache closes. This happens when there are more than one replica for the region. The change checks for cache close after replicating the data; if cache is closed, throws exception back to the accessor/caller, whic

[GitHub] [geode-native] pivotal-jbarrett opened pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-14 Thread GitHub
[ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp opened pull request #2482: GEODE-369: wait for primary to be available when going from 0 to 1 server

2018-09-14 Thread GitHub
Co-Authored-By: Bill Burcham Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in th

[GitHub] [geode] pdxrunner opened pull request #2483: GEODE-5741: Modified tests to be independant of environment

2018-09-14 Thread GitHub
Tests using relative paths had depended on running as a user other than root. The tests expected that file or directory creation would fail because the user would not have permission to write to '/'. A further problem was the several tests had hard-coded '/' in pathnames, which is incompatible wit

[GitHub] [geode] mhansonp commented on pull request #2482: GEODE-369: wait for primary to be available when going from 0 to 1 server

2018-09-14 Thread GitHub
Here is the important change for the primary problem. Elsewhere we cleaned up and normalized most of the timeouts to 3 minutes each, and used Awaitility. [ Full content available at: https://github.com/apache/geode/pull/2482 ] This message was relayed via gitbox.apache.org for notifications@geod

[GitHub] [geode] mhansonp closed pull request #2482: GEODE-369: wait for primary to be available when going from 0 to 1 server

2018-09-14 Thread GitHub
[ pull request closed by mhansonp ] [ Full content available at: https://github.com/apache/geode/pull/2482 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2475: GEODE-3: exclude internal packages when generating javadoc

2018-09-14 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2475 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] rhoughton-pivot commented on issue #2422: GEODE-5688 Create task to generate all sources for better IDE behavior

2018-09-15 Thread GitHub
t; > @@ -61,3 +61,7 @@ task zip(type: Zip) { > } > > assemble.dependsOn 'zip' > + > +afterEvaluate { > > Is this afterEvaluate really necessary too? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it

[GitHub] [geode] Bill commented on pull request #2482: GEODE-369: wait for primary to be available when going from 0 to 1 server

2018-09-16 Thread GitHub
This should say "1 or more". The big learning here was that it's not possible to test for zero connected servers via the API (calling through the API in that case results in the `NoSubscriptionServersAvailableException` exception). [ Full content available at: https://github.com/apache/geode/pul

[GitHub] [geode] Bill commented on pull request #2482: GEODE-369: wait for primary to be available when going from 0 to 1 server

2018-09-16 Thread GitHub
Should throw `IllegalArgumentException` if `connectedServers < 1` or `redundantServers < 0` [ Full content available at: https://github.com/apache/geode/pull/2482 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell opened pull request #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
[ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on issue #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
Can’t wait to take time to review this. Why can’t we have fully autonomous cars already!! [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
Now that the C++ doesn’t lookup this type anymore can’t we just used GetType on Object? [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
Formatting? Looks like tabs? [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] moleske commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
I think this function is unused [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] moleske commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-16 Thread GitHub
Some missed commented code [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-17 Thread GitHub
Indeed we can. Good catch. Done. [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa commented on pull request #354: Geode 5259 remove class id from data serializable

2018-09-17 Thread GitHub
What was wrong with 42? ;) [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey closed pull request #351: GEODE-5626: Fix crash in register all keys

2018-09-17 Thread GitHub
[ pull request closed by pdxcodemonkey ] [ Full content available at: https://github.com/apache/geode-native/pull/351 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey closed pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-17 Thread GitHub
[ pull request closed by pdxcodemonkey ] [ Full content available at: https://github.com/apache/geode-native/pull/352 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
Good catch Michael. [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
Nice catch. [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda closed pull request #2244: GEODE-5338: Geode client to support Trust and Keystore rotation

2018-09-17 Thread GitHub
[ pull request closed by sboorlagadda ] [ Full content available at: https://github.com/apache/geode/pull/2244 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied closed pull request #2466: GEODE-5737: Require gradle version 4.10.1

2018-09-17 Thread GitHub
[ pull request closed by PurelyApplied ] [ Full content available at: https://github.com/apache/geode/pull/2466 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
2 spaces [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
2 spaces [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
I think `PdxType` can go away. It isn't really a `DSCode`. [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
`std::type_info` not "type name" [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
This should be refactored out. It is not really a primitive type, it just pretends to be one. Give it the same treatment we did for `PdxTypeHandler` and `DataSerializableHandler`. In fact rename `PdxTypeHander` to `PdxSerializableHandler`, the current name is confusing with `PdxType`, which is

[GitHub] [geode-native] mmartell commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
I like 7. [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #2476: GEODE-3: small java 9 compatibility fixes

2018-09-17 Thread GitHub
I think we can get rid of this option for all the platforms. This is from java9 documentation: -d32 This option is deprecated and will be removed in a future release. -d64 This option is deprecated and will be removed in a future release. Oracle Solaris, Linux, and OS X: Runs the application in

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Do we have a lot of unused member functions in the test code? What's the extent of this problem? I'd hate to disable this warning if we don't have to... [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notificatio

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Namespace with no name again [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Should we be consistent with the test namespace? I'm not certain there's a significant reason to have both apache::geode::testing and apache::geode::client::testing. Seems to me the one including client is the right way to go, since we're not supposed to be testing geode itself. [ Full conten

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
No-name namespace again. Looks like there are more of these, I'm gonna stop flagging them here. [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Pls remove commented code here and below [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Probably don't need this comment [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Would `= default` work here? [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
... and again [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
I'm not familiar with this construct. An anonymous namespace? Was this intentional? [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Pls remove commented code [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
These comments add no value, let's get rid of em [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner closed pull request #2483: GEODE-5741: Modified tests to be independant of environment

2018-09-17 Thread GitHub
[ pull request closed by pdxrunner ] [ Full content available at: https://github.com/apache/geode/pull/2483 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
It's still used by `DistributedSystem::AppDomainInstanceInitialization` and `TcrMessage`. Will address in separate ticket: https://issues.apache.org/jira/browse/GEODE-5743 [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.or

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Yes! There are several headers that are included that implement lots of methods that are never used in some units. If these were compiled separately as a library the issue would go away but that is way beyond the point of this change right now. The goal is to get the production code clear. So f

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Probably [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Yup! Its a technique to isolate otherwise global namespace `using` or type definitions to the current compilation unit. [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Yup! [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
They don't but this was an auto format change. [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
These comments are logging statements that get uncommented when there is a problem with this tests. Given this isn't production code and to be deprecated tests that I am not going to touch logging comments. [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This messa

[GitHub] [geode-native] mmartell commented on pull request #354: GEODE-5259: Remove class id from data serializable

2018-09-17 Thread GitHub
Will address in separate ticket: https://issues.apache.org/jira/browse/GEODE-5743 [ Full content available at: https://github.com/apache/geode-native/pull/354 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
These comments are logging statements that get uncommented when there is a problem with this tests. Given this isn't production code and to be deprecated tests that I am not going to touch logging comments. This would be a months long effort to get rid of this behavior in all the tests. [ Full

[GitHub] [geode] agingade closed pull request #2481: GEODE-5605: After removeAll/PutAll messages are processed on replicas, check for cache close is done.

2018-09-17 Thread GitHub
[ pull request closed by agingade ] [ Full content available at: https://github.com/apache/geode/pull/2481 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on issue #2480: GEODE-4052: Fix StatusServerExitCodeAcceptanceTest.statusWithWrongPid failure

2018-09-17 Thread GitHub
commit messages fixed [ Full content available at: https://github.com/apache/geode/pull/2480 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout closed pull request #2462: GEODE-5732: Remove TxCommitMessageTest

2018-09-17 Thread GitHub
[ pull request closed by upthewaterspout ] [ Full content available at: https://github.com/apache/geode/pull/2462 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett closed pull request #2422: GEODE-5688 Create task to generate all sources for better IDE behavior

2018-09-17 Thread GitHub
[ pull request closed by pivotal-jbarrett ] [ Full content available at: https://github.com/apache/geode/pull/2422 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa commented on pull request #353: GEODE-5738: Fixes google-global-names-in-headers warning

2018-09-17 Thread GitHub
Sure be nice to get rid of these Helper classes... [ Full content available at: https://github.com/apache/geode-native/pull/353 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

<    1   2   3   4   5   6   7   8   9   10   >