[GitHub] [geode] pdxrunner commented on pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

2018-09-11 Thread GitHub
Why the mixed use of `MAX_OPLOG_SIZE_IN_BYTES` and the literal `10737418240l` from one test to the next? It's not wrong, just looks inconsistent. [ Full content available at: https://github.com/apache/geode/pull/2449 ] This message was relayed via gitbox.apache.org for notifications@geode.apache

[GitHub] [geode] kirklund commented on pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

2018-09-11 Thread GitHub
I copied that from the old test. Several tests use smaller literals and honestly, I just didn't realize that 10737418240l was equal to MAX_OPLOG_SIZE_IN_BYTES (is it?). I'll check if they're the same and change it. [ Full content available at: https://github.com/apache/geode/pull/2449 ] This mes

[GitHub] [geode] jinmeiliao closed pull request #2446: GEODE-5716: add debug capability to the processes started using GfshRule

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

[GitHub] [geode] kirklund commented on pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

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

[GitHub] [geode] pdxrunner commented on pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

2018-09-11 Thread GitHub
Thanks. Wasn't sure it was a holdover from from pre-refactor or intentional for a reason I didn't discern. [ Full content available at: https://github.com/apache/geode/pull/2449 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett opened pull request #2453: GEODE-5723: Don't publish to maven repo unless SNAPSHOT.

2018-09-11 Thread GitHub
Co-authored-by: Dick Cavender Co-authored-by: Jacob Barrett 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

[GitHub] [geode] pivotal-jbarrett commented on issue #2453: GEODE-5723: Don't publish to maven repo unless SNAPSHOT.

2018-09-11 Thread GitHub
@nabarunnag this should fix the publish step in the pipeline for release 1.7.0 if you cherry pick it. [ Full content available at: https://github.com/apache/geode/pull/2453 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao opened pull request #2454: GEODE-5716: simplify process output to stdout/stderr

2018-09-11 Thread GitHub
This also makes the output of the process easier to read. 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

[GitHub] [geode] WireBaron commented on pull request #2452: GEODE-5094: Replace flaky expiration with prexisting better one

2018-09-11 Thread GitHub
This seems more complicated than using the junitparams.Parameter approach. [ Full content available at: https://github.com/apache/geode/pull/2452 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett closed pull request #2424: GEODE-5694 Do not create or publish zip distributions

2018-09-11 Thread GitHub
[ pull request closed by pivotal-jbarrett ] [ Full content available at: https://github.com/apache/geode/pull/2424 ] 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-11 Thread GitHub
Why does this need to be done in afterEvaluate? [ 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] balesh2 opened pull request #2455: GEODE-5712: move awaitility into invoked runnable

2018-09-11 Thread GitHub
Assertion errors within invoked runnables get wrapped in an RMIException, so untilAsserted does not recognize them as failed assertions. Since they are treated as unexpected errors, awaitility does not loop. Moving the awaitility inside the invocation gives us the expected behavior. Signed-off-by

[GitHub] [geode] mcmellawatt opened pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-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? - [X] Has y

[GitHub] [geode] rhoughton-pivot opened pull request #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

2018-09-11 Thread GitHub
Found during testing, relating to GEODE-5694, remove the zip artifacts for better signing behavior Authored-by: Robert Houghton 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 take

[GitHub] [geode] rhoughton-pivot commented on issue #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

2018-09-11 Thread GitHub
@pivotal-jbarrett @smgoller @nabarunnag This changes the signing behavior a little bit, as relates to ZIP artifacts. Please check it out. [ Full content available at: https://github.com/apache/geode/pull/2457 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] nabarunnag closed pull request #2453: GEODE-5723: Don't publish to maven repo unless SNAPSHOT.

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

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

2018-09-11 Thread GitHub
the notifyOnly message to trigger callbacks And computeCompressedShort should not pack inhibitAllNotifications 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

[GitHub] [geode-native] pivotal-jbarrett opened pull request #349: GEODE-5698: Fixes shared_ptr access.

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

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #347: GEODE-5707: CommitConflictException Test case.

2018-09-11 Thread GitHub
Would `EXPECT_THROW` not work in this case? [ Full content available at: https://github.com/apache/geode-native/pull/347 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] moleske commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-11 Thread GitHub
On GitHub this doesn't render as list making reading it more challenging. My understanding is that lettered lists are not valid markdown. I might suggest using either unordered lists, numbers, or using html (since that is all markdown is anyway) [ Full content available at:

[GitHub] [geode] moleske commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-11 Thread GitHub
This is a very Mac specific command, not sure that's what you intended [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett opened pull request #350: GEODE-5730: Fixes google-explicit-constructor clang-tidy warning

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

[GitHub] [geode-native] vaijira commented on pull request #347: GEODE-5707: CommitConflictException Test case.

2018-09-12 Thread GitHub
The problem with EXPECT_THROW or ASSERT_THROW is that i cannot assert that is a CommitConflictException only an apache::geode::client::Exception (at least i couldn't make it work with apache::geode::client::CommitConflictException) and to be sure that is a CommitConflictException i have to exami

[GitHub] [geode-native] pdxcodemonkey commented on issue #348: GEODE-5713: fix valgrind uninitialized value

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

[GitHub] [geode-native] pdxcodemonkey commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
The test code seems very fond of verbose comparisons like this - how about just `(*pIPtr != *newPiPtr)`? [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
Any reason we can't just delete this commented-out code? [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
Same here, `== true` is redundant [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett closed pull request #348: GEODE-5713: fix valgrind uninitialized value

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

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
This looks like the formatter just cleaned up this comment. I'd rather stay away from it in this commit. I used `clang-tidy` to automate the fixes and `clang-format` to cleanup the formatting after. [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was r

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
Again, this was mostly automated. I'd like to keep it that way. [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
Again, this was mostly automated. I'd like to keep it that way. [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on issue #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
@pdxcodemonkey Any objects to keeping the automated minimal changes. I like your other refactoring suggestions but I'd rather do that as a result of other non-automated work. [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache

[GitHub] [geode] kirklund commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
You should remove any mention of "closed" [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
You should remove any mention of "closed" or "open" [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Thanks, I'll clean that up [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Missed that one, will clean it up [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
I'll fix this, thanks [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] davebarnes97 commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
In this case, stick with an ordered list. A set of nested numbers will be OK. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] davebarnes97 commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
In this case, the nested list could be unordered. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 closed pull request #2447: GEODE-5712: Increase Awaitility timeout

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

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Thanks Dave, that's what I settled on as well. See my latest commit. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Missed the list formatting on this one, will fix. Thanks Dave. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

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

2018-09-12 Thread GitHub
I like the idea of this minimal task, but I somewhat agree with Anthony that having to remember to run `./gradlew generate` is little better than having to remember to run `./gradlew dev`. Is there a way to hook this into the `idea` module (and for Eclipse if necessary)? It's been a huge pain

[GitHub] [geode] mcmellawatt opened pull request #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-12 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] jinmeiliao opened pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-12 Thread GitHub
* file saving should be handled by the command's postExecutor to save to appropriate places * after saving file content to a directory already, the ResultModel turned the FileResultModel into InfoResultModel. * gfsh should not be the place to save files in ModelCommandResult. Thank you for submi

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

2018-09-12 Thread GitHub
This change is removed from the PR. This will be addressed as part of separate ticket. [ Full content available at: https://github.com/apache/geode/pull/2450 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

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

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

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

2018-09-12 Thread GitHub
There is not a way to teach IDEA to generate or run arbitrary tasks. You can tell it to use gradle for all tasks or for none. [ 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] pdxcodemonkey opened pull request #351: GEODE-5626: Fix crash in register all keys

2018-09-12 Thread GitHub
- Refactor Cluster class so we can apply locators separately. - Clean up server directories at test startup. - Add test cases for caching proxy and proxy regions with getInitialValues set to true - remove extraneous comment and empty doc comments - throw exception if gfsh exits non-zero - use rela

[GitHub] [geode-native] pdxcodemonkey commented on issue #349: GEODE-5698: Fixes shared_ptr access.

2018-09-12 Thread GitHub
Makes sense, I'm fine with it as is. [ Full content available at: https://github.com/apache/geode-native/pull/349 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pdxcodemonkey closed pull request #349: GEODE-5698: Fixes shared_ptr access.

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

[GitHub] [geode-native] pivotal-jbarrett closed pull request #350: GEODE-5730: Fixes google-explicit-constructor clang-tidy warning

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

[GitHub] [geode] mcmellawatt commented on issue #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Made all the requested changes - thanks for all the feedback! [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
This step is not required as there is no such variable in the gradle build. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
I am missing the referenced b-e above? [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 opened pull request #2461: GEODE-5501: add logging in test base for debug

2018-09-12 Thread GitHub
* Add logging to the JUnit4DistributedTestCase for test lifecycle. The GEODE-5501 bug may be due to tests' @After executing after the next test has started. Extra logging as to when tests are starting and ending will help with diagnosing this failure when it occurs. * Touch the failing test, NetSea

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Good to know - was this deprecated as part of the recent build changes? Will fix. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
Ack yeah this is now 2-5 with the last set of changes. Will fix, thanks! [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund closed pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

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

[GitHub] [geode] pivotal-jbarrett commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-12 Thread GitHub
It never applied to Geode. [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

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

2018-09-12 Thread GitHub
Too many `*` here. This is not a documentation block, just a comment block. [ 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] pdxrunner commented on pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-12 Thread GitHub
Will this test pass on Windows? [ Full content available at: https://github.com/apache/geode/pull/2460 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt closed pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

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

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

2018-09-12 Thread GitHub
Turns out it does not. I had both generation tasks using identical formatting for consistency, but it is not needed here. I have removed the block for this dependency change. [ Full content available at: https://github.com/apache/geode/pull/2422 ] This message was relayed via gitbox.apache.org f

[GitHub] [geode] Bill opened pull request #2462: GEODE-5732: Remove TxCommitMessageTest

2018-09-12 Thread GitHub
After recent move to Mockito, we found that this test doesn't fail when we break the corresponding product code e.g. changing hasFlagsField() to always return true should break tests but doesn't. We verified that the test named like TxCommitMessageBC* do a better job of checking serialization of T

[GitHub] [geode] Bill commented on issue #2462: GEODE-5732: Remove TxCommitMessageTest

2018-09-12 Thread GitHub
@pivotal-jbarrett @nabarunnag @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] pdxrunner commented on pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-12 Thread GitHub
For this and the following test `mytemp` is created in the `tempFolder` so explicit deletion shouldn't be necessary. [ Full content available at: https://github.com/apache/geode/pull/2460 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pdxrunner commented on issue #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-12 Thread GitHub
A more descriptive message as to why this change is desired would be great. As it is, the change looks OK but I wouldn't know why. Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the actual size in bytes of these string values? [ Full content available at: https://gith

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

2018-09-12 Thread GitHub
This code was not in a good state. So the code has been refactored to not only address issues where the code was giving bad feedback, but to address the impressive structural duplication. Things done: -Extracted a bunch of methods -Added new Test files for test subjects that should have been sep

[GitHub] [geode] galen-pivotal opened pull request #2464: GEODE-5734: allow extensibility of Docker build mounts.

2018-09-12 Thread GitHub
If a user wants to extend Gradle's Docker plugin with their own tests, setting dunitDockerVolumes to some value, then that value will be overridden in docker.gradle. For better extensibility, do not set this property if it is already set. [ Full content available at: https://github.com/apache/geo

[GitHub] [geode] jinmeiliao commented on pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-12 Thread GitHub
actually the exported files without absolute paths is written to the user.dir, so we need an explicit delete both here and the test below. The test below is deleting the wrong file. I checked a fix for that. [ Full content available at: https://github.com/apache/geode/pull/2460 ] This message wa

[GitHub] [geode] jdeppe-pivotal commented on pull request #2464: GEODE-5734: allow extensibility of Docker build mounts.

2018-09-12 Thread GitHub
This `if` block used to live in `build.gradle`. I believe the correct fix is to push it back there. Any 'other' projects, pulling in geode would then have the ability to override the `dunitDockerVolumes` variable. As it stands, it is _always_ going to be reset here which is incorrect. [ Full co

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

2018-09-13 Thread GitHub
[ 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] pdxrunner commented on pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-13 Thread GitHub
Thanks. I stand corrected. [ Full content available at: https://github.com/apache/geode/pull/2460 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao closed pull request #2460: GEODE-5727: rework how ResultModel deal with file contents.

2018-09-13 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2460 ] 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-13 Thread GitHub
Auto [ 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-13 Thread GitHub
Obvious comments should be omitted. [ 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-13 Thread GitHub
Auto [ 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-13 Thread GitHub
Formatting seems inconsistent [ 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-13 Thread GitHub
Auto [ 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-13 Thread GitHub
Why muddy the example with types we have to cast? [ 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] mcmellawatt commented on issue #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-13 Thread GitHub
> A more descriptive message as to why this change is desired would be great. > As it is, the change looks OK but I wouldn't know why. > > Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the > actual size in bytes of these string values? @pdxrunner - would "Using <8 bit

[GitHub] [geode] mcmellawatt commented on issue #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-13 Thread GitHub
> A more descriptive message as to why this change is desired would be great. > As it is, the change looks OK but I wouldn't know why. > > Looking at the JIRA, it seems we need the value to be > 8 bytes. What is the > actual size in bytes of these string values? @pdxrunner - would "Using <8 bit

[GitHub] [geode] galen-pivotal closed pull request #2464: GEODE-5734: allow extensibility of Docker build mounts.

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

[GitHub] [geode] upthewaterspout closed pull request #2452: GEODE-5094: Replace flaky expiration with prexisting better one

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

[GitHub] [geode] pdxrunner commented on issue #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-13 Thread GitHub
> > A more descriptive message as to why this change is desired would be great. > > As it is, the change looks OK but I wouldn't know why. > > Looking at the JIRA, it seems we need the value to be > 8 bytes. What is > > the actual size in bytes of these string values? > > @pdxrunner - would "Usi

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

2018-09-13 Thread GitHub
@upthewaterspout @WireBaron @balesh2 Please review when you get the chance. [ Full content available at: https://github.com/apache/geode/pull/2463 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

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

2018-09-13 Thread GitHub
`pf` isn't a great name for such otherwise beautiful new code. [ 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] mcmellawatt commented on issue #2459: GEODE-5725: Using string values to exercise off-heap

2018-09-13 Thread GitHub
Will do, thanks Ken! [ Full content available at: https://github.com/apache/geode/pull/2459 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt closed pull request #2459: GEODE-5725: Using string values to exercise off-heap

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

[GitHub] [geode] PurelyApplied commented on issue #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

2018-09-13 Thread GitHub
Spurious failure ``` > Task :geode-core:test org.apache.geode.internal.process.lang.AvailablePidTest > findAvailablePidShouldNotReturnLivePid FAILED org.junit.runners.model.TestTimedOutException: test timed out after 20 seconds at java.nio.DirectByteBuffer.(DirectByteBuffer.java:162)

[GitHub] [geode] PurelyApplied closed pull request #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

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

[GitHub] [geode] jhuynh1 closed pull request #2433: GEODE-5700: Removed stopping cache server explicitly

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

[GitHub] [geode] jinmeiliao closed pull request #2454: GEODE-5716: GfshRule improvement

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

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

2018-09-13 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-native] pdxcodemonkey commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-13 Thread GitHub
Removed tabs and fixed indentation [ 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] pdxcodemonkey commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-13 Thread GitHub
Fixed [ 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] pdxcodemonkey commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-13 Thread GitHub
Yeah, kinda silly. Fixed types, and fixed in other copies of Order.cpp/Order.hpp in examples. [ 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] pdxcodemonkey commented on pull request #352: GEODE-4338: Add C++ Continuous Query Example

2018-09-13 Thread GitHub
Fixed [ 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-13 Thread GitHub
Formatting? Why did you move from right to left spacing? I believe all the other sources and most of Geode is right spaced. [ 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

  1   2   3   4   5   6   7   8   9   10   >