[infinispan-dev] Proposal for moving Hibernate 2l provider to Infinispan

[Sanne Grinovero]

A couple of comments inline:

On 2 June 2017 at 13:45, Galder Zamarreño <galder at redhat.com> wrote:
> I think it's going through, we've approved you in the past.
>
> Replies below:
>
>> On 31 May 2017, at 17:02, Steve Ebersole <steve at hibernate.org> wrote:
>>
>> Just a heads up - FWIW I doubt my reply goes through to the entire infinispan-dev list.

We're good, I received your answers too.

>> Replies inline...
>>
>>
>> 3. What should be the artifact name? Should it be 'hibernate-infinispan' like it is today? The difference with the existing cache provider would be the groupId. Or some other artifact id?
>>
>> Since you use Maven (IIUC) you could just publish a relocation...
>
> Oh, didn't know about that. Yeah, I think we'd do that:
> https://maven.apache.org/guides/mini/guide-relocation.html

Here an example, from when the Hibernate Search / Infinispan Directory
component was moved to Infinispan:

 - https://github.com/hibernate/hibernate-search/blob/master/infinispan/pom.xml#L23-L30

I guess you'll need an example in Gradle, but at least it shows you
can redirect all of organizationId, artifactId and versions as this
has worked nicely.

It's transparent to the users but they'll get a warning in the build.

>> 4. Should the main artifact contain the hibernate major version it belongs to? E.g. assuming we take 'hibernate-infinispan', should it be like that, or should it instead be 'hibernate5-infinispan'? This is where it'd be interesting to hear about our past Lucene directory or Query integration experience.
>>
>> Probably, but (no promises) one thing I wanted to look at since Hibernate baselines on Java 8, is to maintain the existing SPI using default methods as a bridge.  But failing that, I think your suggestion is the best option.
>>
>>
>> 5. A thing to consider also is whether to maintain same package naming. We're currently using 'org.hibernate.cache.infinispan.*'. From a compatibility sense, it'd help to keep same package since users reference region factory fully qualified class names. We'd also continue to be sole owners of 'org.hibernate.cache.infinispan.*'. However, I dunno whether having 'org.hibernate...' package name within Infinispan repo would create other issues?
>>
>> FWIW Hibernate offers "short naming" or "friendly naming" for many configurable settings, cache providers being one.  For hibernate-infinispan we register 2: "infinispan" and "infinispan-jndi".  You can see this in org.hibernate.cache.infinispan.StrategyRegistrationProviderImpl.  That approach will continue to work when you move it.  The point being that users do not specify the class name in config, they'd just specify "infinispan", "infinispan-jndi", etc.
>
> Ah good to know, I wasn't aware of it. I'll look into that.
>
>> 6. Testing wise, the cache provider is currently tested one test at the time, using JUnit. The testsuite already runs fast enough and I'd prefer not to change anything in this area right now. Is that Ok? Or is there any desire to move it to TestNG?

I wouldn't convert them, unless you have a concrete need to reuse some
helpers, but even so I'd be conservative.
Several other modules within Infinispan are also using JUnit and this
is unlikely to change.

Eventually I'd actually like to discuss moving it all to JUnit 5, but
let's keep that debate for another day.

Thanks,
Sanne

>>
>> Hmmm, that is actually surprising... I thought the hibernate-infinispan  provider tests were still disabled as they had routinely caused intermittent failures of the build.  I guess this was rectified?
>
> They seem pretty stable to me when I run them locally.
>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev