jchen21 commented on a change in pull request #7177:
URL: https://github.com/apache/geode/pull/7177#discussion_r765351757
##########
File path: geode-common/build.gradle
##########
@@ -33,6 +33,9 @@ dependencies {
implementation('com.fasterxml.jackson.core:jackson-databind')
// test
+ testImplementation('com.github.stefanbirkner:system-rules') {
Review comment:
Is the 3rd party library open source license compatible with Geode?
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/lang/SystemPropertyHelper.java
##########
@@ -25,8 +22,10 @@
*/
public class SystemPropertyHelper {
- public static final String GEODE_PREFIX = "geode.";
- public static final String GEMFIRE_PREFIX = "gemfire.";
+ @SuppressWarnings("unused")
+ public static final String GEODE_PREFIX = SystemProperty.GEODE_PREFIX;
Review comment:
Why not remove unused `GEODE_PREFIX`?
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/BrokenSerializationConsistencyRegressionTest.java
##########
@@ -15,8 +15,8 @@
package org.apache.geode.internal.cache;
import static org.apache.geode.cache.RegionShortcut.REPLICATE;
+import static
org.apache.geode.internal.lang.SystemPropertyHelper.DEFAULT_PREFIX;
Review comment:
Why not importing the new `SystemProperty.DEFAULT_PREFIX`? Since the new
`SystemProperty` class is introduced. My understanding is that the new class
`SystemProperty` will be used instead of the existing `SystemPropertyHelper`.
And `SystemPropertyHelper.DEFAULT_PREFIX` is essentially
`SystemProperty.DEFAULT_PREFIX`. I see there are quite a few places in this
pull request that use `SystemPropertyHelper.DEFAULT_PREFIX`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
