+1 to adding a way to copy the RegionAttributes. BTW, I really wish this RegionFactory was an interface. I don't know if adding a copy constructor makes it harder to migrate to an interface later, but maybe just having this single public method might be better?
+ <K, V> RegionFactory<K, V> createRegionFactory(RegionFactory<K, V> regionFactory); On Fri, Dec 13, 2019 at 9:35 AM Mark Hanson <mhan...@pivotal.io> wrote: > Hi Udo, > > I disagree. I believe the currently published best practice is to use > RegionFactory. As a part of the work I have been doing, I have been > migrating code from the AttributesFactory pattern to the RegionFactory > pattern. To support that, I believe a copy constructor for RegionFactory is > necessary. And thus a createRegionFactory > > Hence my changes. > > Thanks, > Mark > > > On Dec 11, 2019, at 4:41 PM, Udo Kohlmeyer <ukohlme...@pivotal.io> > wrote: > > > > I think at this point I'd be looking at the new V2 Management API's for > Regions. > > > > I think any new "public" effort that we'd be adding to the product > should be done through the Management API's for Regions, rather than > exposing new public API's that in reality should not be made "public". > > > > --Udo > > > > On 12/11/19 3:53 PM, Mark Hanson wrote: > >> Basically the point is to allow a use to copy a RegionFactory because > under certain circumstances it is necessary. I found that when migrating > from AttributesFactory. > >> > >> Thanks, > >> Mark > >> > >>> On Dec 11, 2019, at 3:48 PM, Anthony Baker <aba...@pivotal.io> wrote: > >>> > >>> Mark, > >>> > >>> Can you share how the API changes will help the user? > >>> > >>> Thanks, > >>> Anthony > >>> > >>> > >>>> On Dec 11, 2019, at 2:57 PM, Mark Hanson <mhan...@pivotal.io> wrote: > >>>> > >>>> Hi All, > >>>> > >>>> There was a suggestion that since I am making a couple user visible > API changes that I might want to ask the dev list. > >>>> > >>>> Basically I was migrating code from AttributesFactory to > RegionFactory and hit a few snags along the way. > >>>> > >>>> Please see https://github.com/apache/geode/pull/4409 < > https://github.com/apache/geode/pull/4409> specifically Cache.java, > RegionFactory.java, and for extra credit GemfireCacheImpl.java > >>>> > >>>> I have commented at the relevant changes. > >>>> > >>>> Thanks, > >>>> Mark > >
