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
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
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
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
[ 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
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:
[ 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
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:
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
@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
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
[ 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
[ Full content available at: https://github.com/apache/geode/pull/2594 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
* 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
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
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
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
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
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
@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
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
@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
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
@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
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
[ 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
Fix intermittently-failing test:
QueryMonitorDUnitTest.testCacheOpAfterQueryCancel()
by increasing timeout to account for GC pauses.
Also did some test refactoring cleanup:
* clean up DefaultQuery.TestHook interface
* remove dead code
* improve test comments
* remove spurious TODOs
*
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
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
[ pull request closed by pivotal-eshu ]
[ Full content available at: https://github.com/apache/geode/pull/2594 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
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
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
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
@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
created a macro for all_gating_jobs and added "complete" tab
[ Full content available at: https://github.com/apache/geode/pull/2598 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
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
This rearrangement is in service of easier future PR review, when dependencies
are updated to be explicit in each module.
[ Full content available at: https://github.com/apache/geode/pull/2599 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@galen-pivotal, @jinmeiliao please review
[ Full content available at: https://github.com/apache/geode/pull/2601 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@galen-pivotal, @jinmeiliao
[ Full content available at: https://github.com/apache/geode/pull/2601 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ 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
@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
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
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
[ pull request closed by bschuchardt ]
[ Full content available at: https://github.com/apache/geode/pull/2588 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
You'll want to fix the commit message to be "GEODE-: Some description of
work"
[ Full content available at: https://github.com/apache/geode/pull/2597 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
You'll want to fix the PR message to be "GEODE-: Some description of work"
[ Full content available at: https://github.com/apache/geode/pull/2597 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
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
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
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
49 matches
Mail list logo
