[GitHub] [geode] kirklund commented on pull request #6960: GEODE-9562: add system property for redis region name

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

kirklund commented on pull request #6960:
URL: https://github.com/apache/geode/pull/6960#issuecomment-941254758


   > As you work on these system properties, take a look at geode's SystemPropertyHelper. The idea of this was to have one place we could look and find all of the system properties geode has. We never have taken the time to move all of geode's sys props into it but the ones that are in it serve as some good examples.
   > 
   > I don't think we should add the redis sys props to it mainly because we want redis to possibly be its own module. But we could have a single RedisSystemPropertyHelper and we could define in it every system property geode-for-redis has. The helper could also make clear what our naming convention for redis system properties is (just like the geode helper supports both the geode. and gemfire. prefix). You don't need to do this as part of this PR but its something worth considering.
   
   @dschneider-pivotal Some of the system properties are loaded via SystemPropertyHelper, but the class using SystemPropertyHelper has the actual property constant and default value. It's still easy to find all system properties using "find usages" of SystemPropertyHelper. I think that's probably better than hardcoding all of our system properties in SystemPropertyHelper classes.


-- 
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