[GitHub] [geode] jchen21 commented on a change in pull request #7177: GEODE-9879: Extract SystemProperty to geode-common

[GitBox <gi...@apache.org>]

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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org