I was wondering. Why do we require to add this Region.SEPERATOR to be anywhere outside of Region.
Geode-management was purposefully designed NOT to have a dependency on core. Creating a new dependency on a donor module, just means that management module will now start knowing about geode. I suggest if you want to make sure that management also uses a common Region.SEPARATOR, then maybe create a class inside of management for now OR we have to look at management to better understand WHY it requires this knowledge and if there could not be a different implementation to avoid creating a new donor project. The whole idea behind modularity is that modules don't expose their internals. The Region Separator is REGION specific. That knowledge should be kept there. It should not be proliferated around or moved into a "common" module, just because there is a leak. @Donal you are closest to the code, but would it maybe not make more sense to just define that constant and maybe raise a JIRA so that we can address this "leakage" at a later stage? --Udo ________________________________ From: Jacob Barrett <jbarr...@pivotal.io> Sent: Saturday, May 16, 2020 11:02 PM To: dev@geode.apache.org <dev@geode.apache.org> Subject: Re: [PROPOSAL] Move definition of Region separator character to geode-common Probably. Unfortunately we haven’t been very good and cleaning these up and moving forward with a Java modules plan. It’s gunna bite us. > On May 16, 2020, at 8:08 PM, Donal Evans <doev...@pivotal.io> wrote: > > In that case, would it also make sense to move the existing GeodeGlossary > class to org.apache.geode.common.internal, from its current location in > org.apache.geode.util.internal? > >> On Sat, May 16, 2020 at 8:02 PM Jacob Barrett <jbarr...@pivotal.io> wrote: >> >> I am fine as long as you make sure you use a package name that is going to >> be Java 9 modules safe. Two modules cannot export the same package. So if >> geode-commons is going to export org.apache.geode.util I think we will have >> collisions. I suggest org.apache.geode.common. >> >> -Jake >> >> >>>> On May 16, 2020, at 1:23 PM, Donal Evans <doev...@pivotal.io> wrote: >>> >>> I've recently been working on a little side project to replace every use >> of >>> a hardcoded "/" character in region names/paths with a reference to the >>> Region.SEPARATOR constant. I ran into some problems though, since the >>> geode-management module needs to know about the separator character (in >> the >>> Region and Index classes) but does not have a dependency on geode-core, >>> where the character is currently defined. >>> >>> Since the whole point of the exercise is to attempt to provide a single >>> place where the region separator character is defined, pulling the >>> definition down into a module upon which both geode-core and >>> geode-management depend seems like the sensible choice, so I'm proposing >> to >>> create a GeodePublicGlossary class (name entirely up for change) in the >>> geode-common/src/main/java/org/apache/geode/util/ package, moving the >>> definition there, then deprecating the definitions in the Region >> interface >>> in geode-core. >>> >>> To preempt a possible question, there already exists a GeodeGlossary >> class >>> (which defines the GEMFIRE_PREFIX constant), but it's in an internal >>> package, so isn't a suitable place to move the definition of the >> currently >>> user-visible region separator character. >>> >>> Any feedback or suggestions on this idea would be very welcome. >> >>
