[GitHub] [geode-native] pivotal-jbarrett opened pull request #372: GEODE-5770: Fixes clang-analyzer-optin.performance.Padding warning

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

[GitHub] [geode] upthewaterspout commented on pull request #2568: GEODE-5811: Get gradle to cache tomcat and jetty installs

2018-10-08 Thread GitHub
Good point. I switched to using depency-versions.properties and now we are using consistent versions of tomcat. Thanks! [ Full content available at: https://github.com/apache/geode/pull/2568 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #372: GEODE-5770: Fixes clang-analyzer-optin.performance.Padding warning

2018-10-08 Thread GitHub
What exactly does this do? It looks like you've swapped around initialization order and perhaps declaration order of a bunch of member variables in several classes. I'm familiar with having to do this quite often to get rid of compiler warnings when using gcc, but this is Clang and referring t

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #372: GEODE-5770: Fixes clang-analyzer-optin.performance.Padding warning

2018-10-08 Thread GitHub
What this is doing is trying to avoid a lot of unnecessary padding that can bloat the size of you objects in memory. The order is important because certain types will have to pad out to fix the alignment types. For example, I can't have an int that starts after a single boolean without padding o

[GitHub] [geode-native] pivotal-jbarrett closed pull request #372: GEODE-5770: Fixes clang-analyzer-optin.performance.Padding warning

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

[GitHub] [geode-native] pivotal-jbarrett closed pull request #371: GEODE-5766: Fixes google-readability-namespace-comments warning

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

[GitHub] [geode-native] pivotal-jbarrett commented on issue #371: GEODE-5766: Fixes google-readability-namespace-comments warning

2018-10-08 Thread GitHub
Merged with #372 [ Full content available at: https://github.com/apache/geode-native/pull/371 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jujoramos opened pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-09 Thread GitHub
GEODE-5830: Fix `NONE` enum for SSL configuration The `SecurableCommunicationChannel.NONE` enum instance now references the correct constant string, `none`, instead of `NO_COMPONENT`. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution w

[GitHub] [geode] jujoramos opened pull request #2582: GEODE-5838: Fix warnings in javadocs generation

2018-10-09 Thread GitHub
GEODE-5838: Fix warnings in javadocs generation 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: - [X] Is there a JIRA ticket associated with this PR? Is i

[GitHub] [geode-native] pivotal-jbarrett opened pull request #373: GEODE-5690: Cleanup all clang-tidy warnings

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

[GitHub] [geode-native] pivotal-jbarrett commented on issue #373: GEODE-5690: Cleanup all clang-tidy warnings

2018-10-09 Thread GitHub
This is the last of the major clang-tidy fixes. Still working with Travis.ci to get a functional clang-tidy build to enforce these rules. [ Full content available at: https://github.com/apache/geode-native/pull/373 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.o

[GitHub] [geode-native] mmartell closed pull request #363: Geode-5768: Disable parallel execution of cli integration-test2

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

[GitHub] [geode] jinmeiliao commented on pull request #2576: GEODE-5821: exclude geode 1.4 when testing with java 9+

2018-10-09 Thread GitHub
since not much discussion about this on the dev list, looks like people don't care much if we go either way. Can we do this for now? We can always reevaluate if further upgrade tests fail on 9+ and get rid of all old versions. [ Full content available at: https://github.com/apache/geode/pull/257

[GitHub] [geode] dhemery opened pull request #2583: GEODE-5501: Wait for listener to disconnect

2018-10-09 Thread GitHub
In several tests, a listener calls disconnectFromDS() to disconnect its member when a certain event occurs. The tests do not wait for the listener to finish disconnecting. When the tests end, the cleanup also calls `disconnectFromDS()` to disconnect the same member. Within `disconnectFromDS()`

[GitHub] [geode] pdxrunner commented on pull request #2580: GEODE-5212: fix failing StartLocator and Server command DUnit tests

2018-10-09 Thread GitHub
+1 I missed that on my recent changes [ Full content available at: https://github.com/apache/geode/pull/2580 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on pull request #2580: GEODE-5212: fix failing StartLocator and Server command DUnit tests

2018-10-09 Thread GitHub
Although not strictly part of this refactoring, the above test, `testAddJvmOptionsForOutOfMemoryErrors` would be better placed in the `StartServerCommandTest`, as the method being tested is (now a static) in `StartServerCommand`. [ Full content available at: https://github.com/apache/geode/pull

[GitHub] [geode] agingade opened pull request #2584: GEODE-5828: Add dunit test reproducing the issue and incorporate review changes

2018-10-09 Thread GitHub
Co-authored-by: @pivotal-eshu This ticket is already closed by the checkin db8ba67d98d75aeb0e72b55274757e67f69c9c51 This PR is to add a dunit test to reproduce and verify the fix. Also, incorporated the release comments missed out in the previous checkin. - [Yes] Is there a JIRA ticket associat

[GitHub] [geode] kirklund opened pull request #2585: GEODE-5799: Restore StatusLogger level to FATAL

2018-10-09 Thread GitHub
When GEODE-2644 is merged to develop, we should be able to change the StatusLogger level back down to WARN. This will prevent this noise: ``` 2018-10-06 23:19:31,304 DisconnectThread ERROR Attempted to append to non-started appender org.apache.geode.internal.logging.log4j.LogWriterAppender-org.a

[GitHub] [geode] sboorlagadda opened pull request #2586: GEODE-5212: fix timing issues in AfterCompletionTest

2018-10-09 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-native] pivotal-jbarrett opened pull request #374: TRAVIS TESTING DO NOT MERGE

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

[GitHub] [geode] dhemery commented on issue #2583: GEODE-5501: Wait for listener to disconnect

2018-10-09 Thread GitHub
The failures in the DistributedTest run (https://concourse.apachegeode-ci.info/builds/3683) are unrelated. [ Full content available at: https://github.com/apache/geode/pull/2583 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin opened pull request #375: GEODE-5746: Deflakify CLI integration test2

2018-10-09 Thread GitHub
Introduced new classes the use xUnit test framework in a way that mirrors C++ integration-test-2. New tests have been created for the these classes, and existing tests have been retrofitted. Co-authored-by: Blake Bender [ Full content available at: https://github.com/apache/geode-native/pull/3

[GitHub] [geode] jdeppe-pivotal closed pull request #2578: GEODE-5829: Do not bind to both localhost and the actual address of the host

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

[GitHub] [geode] mcmellawatt opened pull request #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

2018-10-09 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: - [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [X] Has y

[GitHub] [geode] mcmellawatt commented on issue #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

2018-10-09 Thread GitHub
We ran into a runtime error where we use the help map in SystemAdmin because of an argument mismatch. This eliminates that problem by using '+' concatenation instead of String.Format. Although '+' concatenation is slightly less performant than String.Format, this is not a concern in help docum

[GitHub] [geode] mcmellawatt commented on issue #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

2018-10-09 Thread GitHub
We ran into a runtime error where we use the help map in SystemAdmin because of an argument count mismatch. This eliminates that problem by using '+' concatenation instead of String.Format. Although '+' concatenation is slightly less performant than String.Format, this is not a concern in help

[GitHub] [geode] upthewaterspout closed pull request #2568: GEODE-5811: Get gradle to cache tomcat and jetty installs

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

[GitHub] [geode] mcmellawatt commented on issue #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

2018-10-09 Thread GitHub
We ran into a runtime error when we use the help map in SystemAdmin because of an argument count mismatch. This eliminates that problem by using '+' concatenation instead of String.Format. Although '+' concatenation is slightly less performant than String.Format, this is not a concern in help

[GitHub] [geode] mcmellawatt commented on issue #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

2018-10-09 Thread GitHub
We ran into a runtime error when we use the help map in SystemAdmin because of an argument count mismatch. This PR eliminates that problem by using '+' concatenation instead of String.Format. Although '+' concatenation is slightly less performant than String.Format, this is not a concern in he

[GitHub] [geode-native] echobravopapa commented on pull request #375: GEODE-5746: Deflakify CLI integration test2

2018-10-10 Thread GitHub
Where's the jar deploy command? [ Full content available at: https://github.com/apache/geode-native/pull/375 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa commented on pull request #375: GEODE-5746: Deflakify CLI integration test2

2018-10-10 Thread GitHub
How does this test run without a jar deploy? [ Full content available at: https://github.com/apache/geode-native/pull/375 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund closed pull request #2585: GEODE-5799: Restore StatusLogger level to FATAL

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

[GitHub] [geode-native] igodwin commented on issue #375: GEODE-5746: Deflakify CLI integration test2

2018-10-10 Thread GitHub
Requested to close until another branch may be merged. Will rebase and reopen. [ Full content available at: https://github.com/apache/geode-native/pull/375 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] igodwin closed pull request #375: GEODE-5746: Deflakify CLI integration test2

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

[GitHub] [geode-native] mmartell opened pull request #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

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

[GitHub] [geode] mcmellawatt closed pull request #2587: GEODE-5844: Eliminating String.Format() calls from initHelpMap()

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

[GitHub] [geode-native] pivotal-jbarrett closed pull request #373: GEODE-5690: Cleanup all clang-tidy warnings

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

[GitHub] [geode-native] pivotal-jbarrett closed pull request #367: TESTING TRAVIS DO NOT MERGE

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

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-10 Thread GitHub
This should be made a constant, `SecurableCommunicationChannels.NONE`, to be consistent with the other declarations. [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on issue #374: GEODE-5693: Use clang-tidy in travis build.

2018-10-10 Thread GitHub
See output from travis build. Sadly it takes about 2h to run. I was able to get travis.ci to increase our build timeout to accommodate. Hopefully as we improve includes that build time will improve. [ Full content available at: https://github.com/apache/geode-native/pull/374 ] This message was

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-10 Thread GitHub
This should be made a constant, `SecurableCommunicationChannels.NONE`, to be consistent with the other declarations. I think I was wrong about this -- however, we have the other channels supported as a comma-separated list, and I don't think it makes sense to have `none` included as a member of

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

2018-10-10 Thread GitHub
Tests shouldn't be writing things to console. Shouldn't there be some flag this call is setting so that you can assert it was called later? [ Full content available at: https://github.com/apache/geode-native/pull/376 ] This message was relayed via gitbox.apache.org for notifications@geode.apache

[GitHub] [geode-native] echobravopapa commented on pull request #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

2018-10-10 Thread GitHub
We'll clean that up [ Full content available at: https://github.com/apache/geode-native/pull/376 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa commented on issue #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

2018-10-10 Thread GitHub
This was meant to go in before the test framework changes in GEODE-5746, which resolves this old way... [ Full content available at: https://github.com/apache/geode-native/pull/376 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] rhoughton-pivot closed pull request #2557: Add JDK version loop for concourse test jobs

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

[GitHub] [geode] bschuchardt opened pull request #2588: GEODE-5843 ReconnectDUnitTest.testReconnectWithRequiredRoleRegained i…

2018-10-10 Thread GitHub
…s being ignored Two problems have been addressed: First, the recursive reconnect logic in InternalDistributedSystem was broken. After recursion created a new cache the higher up reconnect() method did not handle things correctly and called things like createAndStartCacheServers() on the

[GitHub] [geode-native] echobravopapa commented on issue #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

2018-10-10 Thread GitHub
@pivotal-jbarrett those debug statements are cleaned up and GEODE-5746 will take care of your other concerns. [ Full content available at: https://github.com/apache/geode-native/pull/376 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] agingade closed pull request #2584: GEODE-5828: Add dunit test reproducing the issue and incorporate review changes

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

[GitHub] [geode-native] echobravopapa closed pull request #376: GEODE-5768: New ContinuousQuery tests using DataSerializable objects

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

[GitHub] [geode] WireBaron opened pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-10 Thread GitHub
…ng sleeps Refactored several tests to use Awaitility instead of sleeping. Removed other sleep calls that were not impacting their test case. 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] gesterzhou opened pull request #2590: GEODE-5252: handleCacheRemoval() could wait for distribution's replies.

2018-10-10 Thread GitHub
Should not block the system notifications for membership changes. @boglesby @sboorlagadda Co-authored-by: Sai Boorlagadda 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: #

[GitHub] [geode] rhoughton-pivot commented on pull request #2576: GEODE-5821: exclude geode 1.4 when testing with java 9+

2018-10-10 Thread GitHub
The fact that we can't run this test means the product also could not be run this way, which is a known and accepted limitation (users MUST upgrade geode BEFORE upgrading java). So skipping this test is the right thing to do. [ Full content available at: https://github.com/apache/geode/pull/257

[GitHub] [geode-native] pivotal-jbarrett opened pull request #377: GEODE-5847: Integrate clang-format into CMake.

2018-10-10 Thread GitHub
Depends on #374 [ Full content available at: https://github.com/apache/geode-native/pull/377 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda commented on pull request #2590: GEODE-5252: handleCacheRemoval() could wait for distribution's replies.

2018-10-10 Thread GitHub
As an alternative, I am going to try reducing the scope of read-write lock. The lock is guarding the state inside `ManagementAdapter` as `handleCacheRemoval` is setting them to `null` [ Full content available at: https://github.com/apache/geode/pull/2590 ] This message was relayed via gitbox.ap

[GitHub] [geode] gesterzhou closed pull request #2590: GEODE-5252: handleCacheRemoval() could wait for distribution's replies.

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

[GitHub] [geode] gesterzhou commented on pull request #2590: GEODE-5252: handleCacheRemoval() could wait for distribution's replies.

2018-10-10 Thread GitHub
without knowing any potential issue, reducing the scope could cause some other hydra tests to fail. We can watch and see. [ Full content available at: https://github.com/apache/geode/pull/2590 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] onichols-pivotal opened pull request #2591: GEODE-5848 conditionally add --add-opens for test jvm versions 9 and up

2018-10-11 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has y

[GitHub] [geode] jujoramos commented on issue #2582: GEODE-5838: Fix warnings in javadocs generation

2018-10-11 Thread GitHub
Thanks @galen-pivotal! You're right, it doesn't even make sense to reference the other example (`SimpleSecurityManager`) so I'll remove that part entirely from the javadocs. Cheers. [ Full content available at: https://github.com/apache/geode/pull/2582 ] This message was relayed via gitbox.apache

[GitHub] [geode] jujoramos commented on issue #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
Agreed @galen-pivotal, I'll remove the `none` constant altogether and update the docs + tests. Cheers. [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell opened pull request #378: GEODE-5638: Move cppcache/integration-tests to legacy

2018-10-11 Thread GitHub
This change puts all of the old framework cpp integration tests into a legacy subdirectory to make it easier to find the other test related projects. [ Full content available at: https://github.com/apache/geode-native/pull/378 ] This message was relayed via gitbox.apache.org for notifications@ge

[GitHub] [geode] sboorlagadda closed pull request #2586: GEODE-5212: fix timing issues in AfterCompletionTest

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

[GitHub] [geode] metatype commented on pull request #2591: GEODE-5848 conditionally add --add-opens for test jvm versions 9 and up

2018-10-11 Thread GitHub
Just curious, is this related to adding the JAXB dependencies or does it solve a different issue? [ Full content available at: https://github.com/apache/geode/pull/2591 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] onichols-pivotal commented on pull request #2591: GEODE-5848 conditionally add --add-opens for test jvm versions 9 and up

2018-10-11 Thread GitHub
this one is related to LogService, I believe [ Full content available at: https://github.com/apache/geode/pull/2591 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
Does removing this not make the test flaky? Seems like it would have the potential to fail on that verify. I have the same question about removing lines 370 and 443 (in the old file's numbering) [ Full content available at: https://github.com/apache/geode/pull/2589 ] This message was relayed via

[GitHub] [geode] balesh2 commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
You could use "untilAsserted" here. You would have to use a soft assertion if you wanted to see all three of those values every time, but even if you just use regular assertions you'll get an error message that is a little more informative (as it stands, you'll just see "expected true and was fa

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
Good cleanup here. However, per [the style guide](https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide), "Always use braces, even around one-line `if`, `else` and other control statements." [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
Suuuper minor grammatical suggestion: put a comma after "all", so that there are clearly three commma-separated options. [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] WireBaron commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
No, the suspect state we're checking is set synchronously in the suspect call above. I'm not sure why this sleep was ever here. InstallView also synchronously sets all the state needed for the test, making the old sleeps at 370 and 443 similarly pointless. [ Full content available at: https:/

[GitHub] [geode] WireBaron commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
Testing this it looks like awaitility swallows the assertion failure and simply outputs: ```org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a lambda expression in org.apache.geode.distributed.internal.membership.gms.fd.GMSHealthMonitorJUnitTest null within 1000 mil

[GitHub] [geode] WireBaron closed pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

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

[GitHub] [geode] onichols-pivotal opened pull request #2592: GEODE-5851 use gradle-dockerized-test-plugin compiled with Java 8

2018-10-11 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has y

[GitHub] [geode] PurelyApplied opened pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has y

[GitHub] [geode] pivotal-eshu opened pull request #2594: GEODE-5849: Do not wrap CacheClosedException if client cache is closing.

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

[GitHub] [geode] mhansonp opened pull request #2595: GEODE-5853: fix to check if testJVM is empty

2018-10-11 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] PurelyApplied commented on pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 Thread GitHub
Any opinions on where this data container class should live? I kept it on the path to be the same as the test that needs it, but that's not really necessary. It could belong to `geode-dunit` as a shared resource, if we track down other places that need a simple `java.io.Serializable` data cont

[GitHub] [geode] mhansonp commented on issue #2595: GEODE-5853: fix to check if testJVM is empty

2018-10-11 Thread GitHub
@pivotal-jbarrett @onichols-pivotal Please review this build fix. [ Full content available at: https://github.com/apache/geode/pull/2595 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 Thread GitHub
Similar to the other comment, there's no real reason this has to get packaged in the `jar` to the full `o.a.g.m.i.c.c` package. Would this test read cleaner if the jar put the `SerializableNameContainer` at the top-level package? I know I'm overthinking this. [ Full content available at: https

[GitHub] [geode] galen-pivotal closed pull request #2595: GEODE-5853: fix to check if testJVM is empty

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

[GitHub] [geode] dschneider-pivotal commented on pull request #2594: GEODE-5849: Do not wrap CacheClosedException if client cache is closing.

2018-10-11 Thread GitHub
why do you have two assignments of "runtimeException"? Seems like the second assignment just throws away your new one. [ Full content available at: https://github.com/apache/geode/pull/2594 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 Thread GitHub
Upon immediate hindsight, this is absolutely not the right way to do this. Maybe examine the class, get a list of dependencies, and depend on those. As it stands, our `repeat-new-tests.sh` will run the entire `*Test` instead of just repeating the changed tests within that scope. [ Full conten

[GitHub] [geode-native] echobravopapa closed pull request #374: GEODE-5693: Use clang-tidy in travis build.

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

[GitHub] [geode] galen-pivotal commented on issue #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
@jujoramos if you want, I'm happy to add the braces and merge. Just figured it might be helpful to get feedback on the style guide. [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] nabarunnag opened pull request #2596: GEODE-5852: Adding 1.7.0 to old version

2018-10-11 Thread GitHub
* as 1.7.0 has been released, this version must be tested against the current snapshot * thus 1.7.0 has been added to the build.gradle file of the geode-old-version project. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contrib

[GitHub] [geode] sboorlagadda commented on pull request #2580: GEODE-5212: fix failing StartLocator and Server command DUnit tests

2018-10-11 Thread GitHub
Done! [ Full content available at: https://github.com/apache/geode/pull/2580 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dickcav closed pull request #2591: GEODE-5848 conditionally add --add-opens for test jvm versions 9 and up

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

[GitHub] [geode-native] mmartell commented on issue #377: GEODE-5847: Integrate clang-format into CMake.

2018-10-11 Thread GitHub
This looks awesome! I assume this corrects any IDE formatting issues so that as long as I compile before I check-in, they will be corrected. [ Full content available at: https://github.com/apache/geode-native/pull/377 ] This message was relayed via gitbox.apache.org for notifications@geode.apach

[GitHub] [geode] pivotal-eshu commented on pull request #2594: GEODE-5849: Do not wrap CacheClosedException if client cache is closing.

2018-10-11 Thread GitHub
You are right, it should not set runtimeException as getCancelCriterion().generateCancelledException would throw exception if cache is closing. I was just use the code in basicPutAll. [ Full content available at: https://github.com/apache/geode/pull/2594 ] This message was relayed via gitbox.apa

[GitHub] [geode] mhansonp opened pull request #2597: Geode 5728

2018-10-11 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 #2597: Geode 5728

2018-10-11 Thread GitHub
Important part of the fix here... [ Full content available at: https://github.com/apache/geode/pull/2597 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp commented on issue #2597: Geode 5728

2018-10-11 Thread GitHub
@WireBaron @galen-pivotal @upthewaterspout [ Full content available at: https://github.com/apache/geode/pull/2597 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu commented on pull request #2594: GEODE-5849: Do not wrap CacheClosedException if client cache is closing.

2018-10-11 Thread GitHub
Actually, the method may not throw CacheClosedException but return CacheClosedException instead. Made the change necessary. [ Full content available at: https://github.com/apache/geode/pull/2594 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on issue #377: GEODE-5847: Integrate clang-format into CMake.

2018-10-11 Thread GitHub
If clang-format is found, yes. [ Full content available at: https://github.com/apache/geode-native/pull/377 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mhansonp commented on issue #2597: Geode 5728

2018-10-11 Thread GitHub
@WireBaron @galen-pivotal @upthewaterspout [ Full content available at: https://github.com/apache/geode/pull/2597 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] onichols-pivotal opened pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 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: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has y

[GitHub] [geode] upthewaterspout commented on pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 Thread GitHub
Should this be `java_test_version.name.endsWith("JDK11")` ? [ Full content available at: https://github.com/apache/geode/pull/2598 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 Thread GitHub
I almost hate to comment on one-line PRs, but if the intent is "only the things that act as blocking for `UpdatePassingRef`, then we should use the same as that block: ``` - name: UpdatePassingRef- name: UpdatePassingRef public: truepublic: true serial: trueserial: true

[GitHub] [geode] PurelyApplied commented on pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 Thread GitHub
I almost hate to comment on one-line PRs, but if the intent is "only the things that act as blocking for `UpdatePassingRef`, then we should use the same as that block: ``` - name: UpdatePassingRef public: true serial: true plan: - get: geode passed: {% for test in tests if not (t

[GitHub] [geode] sboorlagadda closed pull request #2580: GEODE-5212: fix failing StartLocator and Server command DUnit tests

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

[GitHub] [geode] dickcav commented on pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 Thread GitHub
@pivotal-amurmann may want to review this. [ Full content available at: https://github.com/apache/geode/pull/2598 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dickcav commented on pull request #2598: GEODE-5855 hide jobs that are not gating in default pipeline view

2018-10-11 Thread GitHub
@pivotal-amurmann Should we then have an All or Complete so we can see them all? [ Full content available at: https://github.com/apache/geode/pull/2598 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

<    3   4   5   6   7   8   9   10   11   12   >