Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/283
IMO losing your gfsh history on an upgrade is probably not a big deal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/278
+1 Looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/commit/c1216eb1817df3c6b20afbaff49ba30e76d34b44#commitcomment-1965
@vectorijk Shoot, I just updated the version to 1.0.0-incubating (no
SNAPSHOT) to get the build to pass before I
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/278
+1 Looks good to me after addressing Kirk's comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/277
+1 Looks good to me. I'll merge this. Thanks!
The travis failure appears to be code checked in on develop with formatting
changes. I'm looking into that, but it's
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/274
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/267
Changes look good. I think there needs to be some changes to
geode-site/website/README.md to instruct people to build the docs and include
them in the website though. Otherwise
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/266
The travis CI is failing because the asf-site branch is just the website,
it doesn't have any code or the .travis.yml file. I wouldn't worry about that.
We don't want
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/259
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/259
@joeymcallister - Ok, the parallel directories makes more sense now. We can
always move stuff around after 1.0 if we decide to.
I also had to do bundle install
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/259
I think geode-docs should be a subdirectory of geode-book so we don't have
two unrelated directories for these docks.
It should probably output to a directory called build
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/259#discussion_r83334338
--- Diff: geode-docs/README.md ---
@@ -1,93 +1,53 @@
-# Project Geode End-User Documentation
+# Apache Geode End-User Documentation
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/235
It would be nice to rename this test to something other than
Bug37377DUnitTest, since no one knows what that means.
---
If your project is set up for it, you can reply
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/225
+1 I'll merge this in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/217
This change is mixing internal classes in the public API. You're adding
getDefinedIndexes to the public API, but it returns LuceneIndexCreationProfile
objects which is an internal
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/190
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/178
+1 looks good. One minor thing - I think could might result in hasNext()
returning true followed by next() returning page with no elements if the last
element gets deleted
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/177
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/169
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/171
Hi William,
Thanks for doing all of this work! This looks like a good start for
examples. I have a few comments below.
1. Did you look into using the gradle
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/168
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/156
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/147#issuecomment-221422193
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/146#issuecomment-221376610
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/144#issuecomment-219785289
Should this stuff be added to the .gitignore as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/141#issuecomment-217282247
+1 I'll merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/139#issuecomment-216614483
+1 looks good, but maybe we should scale this test way down. Why does it
need to put this much data in order to test this feature? Could we bring
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/137#issuecomment-214898386
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/130#issuecomment-212638825
I also created a JIRA for these changes - feel free to update:
https://issues.apache.org/jira/browse/GEODE-1269
---
If your project is set up
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/130#issuecomment-212638738
I put this pull request on the feature branch feature/GEODE-1269 so we can
work on it together to fix some of these failures.
---
If your project
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/130#issuecomment-211511367
Thanks for sending this code Asif!
I ran a precheckin, and there are some failing tests.
*ResourceManagerDUnitTest
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/111#issuecomment-193948459
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/110#issuecomment-193924869
+1
You should rebase your branch on apache develop. It looks like what
happened is that your previous pull request was cherry-picked to develop
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/100#issuecomment-186420300
+1
I'm curious about why you left the json.org code alone, but not
gemfire-joptsimple. Not a big deal because either way I think we have plans
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/98#issuecomment-185867270
BTW - I can push this change myself, I just thought it would be easier to
review as a PR than a reviewboard request. If I get a couple of +1s and no -1s
GitHub user upthewaterspout opened a pull request:
https://github.com/apache/incubator-geode/pull/98
GEODE-866: Converting calls of vm.invoke to lambdas
I automatically converted many of the calls of vm.invoke(Class, String) to
use a lambda instead, as discussed. Please review
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/43#issuecomment-181666323
Looks like these {@linkplain } links are causing javadoc warns when this is
merged to the latest develop:
./gradlew gemfire-core:javadoc
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/80#issuecomment-175780734
Looks good! One minor point - I think this is missing something that will
tell gradle what the copyPulsePropFile task depends on. You could either add
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/74#issuecomment-173369343
This is GEODE-393. I discussed this with Shreedhar, these changes need some
tests and then they should be good to merge. I'll pick this up.
---
If your
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/62#issuecomment-165910633
Is there are reason why this test should be disabled? I thought our
practice is to not disable failing tests?
If you do need to disable the test
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/58#issuecomment-164080920
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/37#issuecomment-161128882
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-155849235
I hit some failures with these changes. I think these related to tests that
are using these features that probably need to get fixed
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-154219832
Looks good! I noticed you left in the localProperties and globalProperties
fields on PartitionAttributesImpl, as well as leaving the toData/fromData
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/26#issuecomment-151952966
The changes in your last commit look good. But something is odd with this
PR, you have some commits from develop that have been cherry picked onto your
45 matches
Mail list logo
