Remember, if there are any concerns about recent backwards-compatible
changes to Geode user APIs, they should be brought on the dev list.
Also, backward-compatible changes are by definition safe and ok for a user
API because it won't break the user's code.
Here's an example of a user API that I recently fixed...
The ClientRegionFactory is a builder with methods that are supposed to
follow fluent-style API (ie, return the ClientRegionFactory instance so you
can chain the calls).
Whoever added setConcurrencyChecksEnabled goofed up and made the return
type void. Changing void to ClientRegionFactory is a fully backwards
compatible change which corrects the API. Since this fixes the API and
doesn't actually change the user API, this should be very safe and improves
Geode by correcting a broken API:
*diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java
b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
*index add35f01a2..2a08307adc 100644*
*---
a/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
*+++
b/geode-core/src/main/java/org/apache/geode/cache/client/ClientRegionFactory.java*
@@ -216,7 +216,7 @@ public interface ClientRegionFactory<K, V> {
* @since GemFire 7.0
* @param concurrencyChecksEnabled whether to perform concurrency checks
on operations
*/
- void setConcurrencyChecksEnabled(boolean concurrencyChecksEnabled);
+ ClientRegionFactory<K, V> setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled);
/**
* Sets the DiskStore name attribute. This causes the region to belong
to the DiskStore.
*diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
*index 64256e8f8e..920deba055 100644*
*---
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
*+++
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ClientRegionFactoryImpl.java*
@@ -186,8 +186,9 @@ public class ClientRegionFactoryImpl<K, V> implements
ClientRegionFactory<K, V>
}
@Override
- public void setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled) {
+ public ClientRegionFactory<K, V> setConcurrencyChecksEnabled(boolean
concurrencyChecksEnabled) {
this.attrsFactory.setConcurrencyChecksEnabled(concurrencyChecksEnabled);
+ return this;
}
@Override
Does anyone have concerns over fixing "void setConcurrencyChecksEnabled"
by changing it to "ClientRegionFactory setConcurrencyChecksEnabled"? If
there is a concern, is the concern about the change itself or because this
was fixed without following a more heavy-weight process?
Thanks,
Kirk
