Ticket reopened in order to implement the changes as described in the
"Improvements on client Function execution API" Proposal in the
https://cwiki.apache.org/confluence/display/GEODE/
[ Full content available at: https://github.com/apache/geode/pull/3888 ]
This message was relayed via
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
I think this mix of camel case and underscores was intentional to help
highlight the three sections (given, when, then). @dhemery may have some
helpful input on this.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
As this test uses given-when-then style, I find its easy to understand what the
test is doing by separating the statements, but I understand its not the
standard in all the tests, so I can change it.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was
This PR is based on GEODE-5222, and in that PR it was considered ok to modify
the interface to add "getDiskUsagePercentage" without default implementation, I
suppose it was due to its not usually implemented as you said.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
@jujoramos Does the addition of a new method on an interface break old code
that is already compiled against the old interface? When you say it "breaks
backward compatibility" what do you mean? Is it that the old class files will
no longer run with the new product jar? Or is it that old code
Since you're already modifying the test name, can you please remove the
underscores and use camel case instead?.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Since you're already modifying the test name, can you please remove the
underscores and use camel case instead?.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The name should read something like
`givenDiskStoreIsCreatedWithMaxSizeThenDiskUsagePercentageShouldBeZeroAndDiskFreePercentageShouldBe100`.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I'm not sure why we didn't consider "worth it" adding a `default`
implementation for `getDiskUsagePercentage` in
[GEODE-5222](https://issues.apache.org/jira/browse/GEODE-5222), I'm tagging the
original reviewers as they might have more context than me on this one.
Either way, unless there's a
@jujoramos I like the idea of adding a default to interface methods we add to
existing external interfaces.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Geode exposes much of the external apis on interfaces. Many of those interfaces
are not meant to be implemented by geode users. Instead they are used to hide
the internal geode implementation. For example Region is an interface that a
geode user never needs to implement. So in the past as we
This is a public interface, even though I rarely see people manually
implementing it, the addition of a new method would break backward
compatibility... would it make sense to add a `default` implementation?.
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message
I have added default implementation for both methods. I suppose the default
implementation of `getDiskUsagePercentage` should be included in 1.10 before
release, right?
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
Same
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
This test moved to PartitionedRegionTest.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Removed FQN
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
This test moved to the unit test PartitionedRegionTest because it didn't need
to be an integration test after updating it.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Again, this is Geode 1.11.
[ Full content available at: https://github.com/apache/geode/pull/3888 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I think the underscores help make the test name more readable in this case
(because of the given/when/then style and the long length of the name).
[ Full content available at: https://github.com/apache/geode/pull/4067 ]
This message was relayed via gitbox.apache.org for
The directions still show how to configure Copyright in preferences of IntelliJ
at https://github.com/apache/geode/blob/develop/BUILDING.md. If you do the
same, your IntelliJ will also produce the same changes that mine did for this
PR.
[ Full content available at:
It is still relevant but I'll delete it. Thanks!
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I pushed more fixes to the javadocs on all of these tests/
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I pushed more fixes to the javadocs on all of these tests.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
> I'm not sure why we didn't consider "worth it" adding a `default`
> implementation for `getDiskUsagePercentage` in
> [GEODE-5222](https://issues.apache.org/jira/browse/GEODE-5222), I'm tagging
> the original reviewers as they might have more context than me on this one.
I think it's a good
LogService is an internal API used by modules other than geode-core. In a
follow-up PR, I will need to move LogService to a User API package. It returns
log4j-api Logger and is an API.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via
Others may not have IntelliJ copyright configured. I added directions to the
Apache Geode README to make it identical to my setup but someone may have
deleted it. My guess is that Jake deleted too much. But yes, I wanted these
changes. I don't want to have to make the .xml files use a different
I don't think either SortLogFile or MergeLogFiles moved so I'm not sure why
IntelliJ made this change. Looks ok although I generally prefer to not add
`@see` javadocs -- I usually just delete them.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed
Before add method is used, array is allocated, with enough capacity, so it
should never happen that we don`t have capacity. If it happens, it is fault.
[ Full content available at: https://github.com/apache/geode/pull/4002 ]
This message was relayed via gitbox.apache.org for
Looks like we have a few lingering test failures, but otherwise I think we can
get this in and iterate on it to address membership's dependencies on logging
related classes in geode-core.
[ Full content available at: https://github.com/apache/geode/pull/4003 ]
This message was relayed via
@mivanac It is your PR after all. Your local build should be failing too. The
`Version` class and some of the internal serialization stuff have moved.
[ Full content available at: https://github.com/apache/geode/pull/4002 ]
This message was relayed via gitbox.apache.org for
I added InetAddressUtilsWithLogging which wraps two of the utils with versions
that log call stack. I also updated all uses of both InetAddressUtils classes
to use `import static`
[ Full content available at: https://github.com/apache/geode/pull/4020 ]
This message was relayed via
@mivanac It doesn't look like these test failures are your fault. Try merging
the latest develop into this branch and pushing again.
[ Full content available at: https://github.com/apache/geode/pull/4002 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@kirklund - Regarding the public APIs - If you feel strongly about adding these
new public API classes, I think it would be helpful to start a discussion on
the dev list about them. This seems like a lot of new public API to be hidden
in a PR that many people will not notice.
[ Full content
@upthewaterspout @jxblum @pivotal-jbarrett @mcmellawatt @nvpivot @metatype One
of the main areas that Geode has not done well in is integration and fostering
non-Pivotal contributes. I believe our continued use and support of "internal"
APIs with a lack of real SPIs is the main reason behind
It looks like my recommendation to use `emptySet()` is causing some test
failures. Looks like the consumer of this method expects to modify the set
returned.
```
...
final int arrayLength = length(bucketSet);
final Set resultSet = new HashSet(arrayLength);
for (int i = 1; i <=
GREEN precheckin results
[ Full content available at: https://github.com/apache/geode/pull/4024 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Note: I'm still very committed to having geode-log4j in the classpath for
integrationTests which do spin up servers/locators, perform logging and
alerting, etc. These tests are not debuggable without logs. We would also have
to find a new home for all integrationTests which have validation
@upthewaterspout I'm committed to properly exposing SPI classes as
non-internal. I also intend to finish up my geode-logback repo which implements
a logback provider with a corresponding blog article. I've discussed this
(having non-internal SPIs) with other Geode committers and most developers
@upthewaterspout @jxblum @pivotal-jbarrett @mcmellawatt @nvpivot @metatype
@demery-pivotal:
One of the main areas that Geode has not done well in is integration and
fostering non-Pivotal contributions. I believe our continued use and
preference of "internal" APIs as integration points with a
40 matches
Mail list logo
