On 26 March 2014 20:18, Hardy Ferentschik <ha...@hibernate.org> wrote: > > On 26 Jan 2014, at 20:52, Sanne Grinovero <sa...@hibernate.org> wrote: > >> The classes in the ORM module are extremely widely used by our primary >> target: >> >> - FullTextQuery >> - Search >> - FullTextSession >> >> and two more lesser ones, but I think the 3 above are probably "THE >> API”. > > That was my thinking as well. > >> To move these out of the way we'd at least need to postpone the >> OSGi work for after a Beta to include a deprecated version of these. > > ? I don’t get what you are saying here. Deprecate it in an Alpha in order > to then move them in a Beta release? That makes no sense what soever to me. > My take on this is, that we are dealing with a new major version of Search > and that we are still in Alpha phase. If we need to make non backwards > compatible changes we should do it now.
If we need to change public API we have the obvious options: 1# just break the API 2# deprecate the old one I didn't mean that we stricly should do 2#, but we should at least try hard ;-) But *if* we go for 2#, we should keep the deprecated classes around for at least a Beta release, or the effort (of keeping them) is not worth it as I think an Alpha release is not going to inspire enough confidence, or have enough visibility, for us to suggest using it as a migration milestone. Problem is, that if we keep them you might not be able to finish the OSGi issue, as it requires these classes to be removed. Or maybe we can relax this requirement temporarily? I guess the important milestone to aim at is to define the final API? I guess we shouldn't necessarily aim at fully resolving HSEARCH-1465 in a single tag, and if it comes to breaking our APIs I'd prefer we consider alternatives carefully. >> Or we keep them around in the backwards compatible module >> "hibernate-search” ? > > For what? Making things even more complicated? I agree with you: I'm not suggesting we actually do it, I'm listing our options: some I like, some I don't (like this one). You can consider it a rethorical figure which stresses the fact the previous alternative is better. > >> On the other option, in Engine we have: >> - Environment (containing all the configuration constants) >> - ProjectionConstants (more useful constants) >> - SearchException >> - SearchFactory (another strong case of public API) >> >> Also to remember the changes in Engine also affect the HQL parser >> (OGM) and Infinispan Query and its users; we can adapt for it but >> ultimately that's our responsibility too, probably best to verify >> early on to see the impact. Not really a reason to not do it, but it >> has an impact on several other projects which we need to facilitate. > > Mind you, we are talking just about an import statement change. No API > or functional change. And yes, our downstream projects would need to adjust. I get that. Still a nice self-documenting deprecated class is so much easier to handle than an import statement change. >> I think I like Hardy's approach of moving from Engine better; not >> least we can provide alternative deprecated constants in the ORM >> module for ORM users to use as a migration help (Environment and >> ProjectionConstants). > > ? You could make a copy of Environment and ProjectionConstants in the ORM module, and deprecate it to document the new position. Maybe we shouldn't invest too much into backwards compatibility, but since this is a trivial step we should just do it. >> To reply on the more fine-grained details: >> >>> org.hibernate.search.Environment -> >>> org.hibernate.search.cfg.Environment >> >> since it's API, WDYT of a fully fledged mouthfull >> “org.hibernate.search.configuration.Environment" > > I like it. In fact I wish cfg would have been called configuration in the > first place. > Having both, cfg and configuration seems odd to me. In this case I prefer to > just > use cfg Same here. I almost suggested to rename all of _cfg_ to _configuration_ but I didn't realize it contained so much public API, so +1 to stick with cfg. >>> org.hibernate.search.SearchFactory -> >>> org.hibernate.search.spi.SearchFactory >> >> as Emmanuel said, that's an API currently. > > Right. I am not 100% happy with this choice either. Mainly moved it there, > since we have SearchFactory > related classes in there already (not all of which are api either) > >> I'm inclined to think we >> should convert it to an SPI, and provide a richer more ORM specific >> version in the ORM module? > > Interesting. On the other hand, making SearchFactory an spi sounds strange to > me. I meant conceptually, the integrators don't really need SearchFactory at all: they rely on SearchFactoryIntegrator. In practice Infinispan Query also exposes the SearchFactory as an API but they could expose an alternative definition for the same functional role. If we break this link, we have the added benefit of being able to expose ORM-specific methods on the public API SearchFactory which we would be bundling in the hibernate-search-orm package (rather than engine as it's today). I don't say there is anything that comes to mind urgently needing this, but it seems nice in terms of future flexibility. The tricky part is that today SearchFactoryIntegrator extends SearchFactory, but since SearchFactoryIntegrator needs to say in Engine, it can't depend on SearchFactory. What would you think of breaking this parent-child relation? From a user's perspective they wouldn't be able to narrow down their SearchFactory to an SearchFactoryIntegrator, but we could provide an unwrap backdoor. I'm actually thinking now that I'd like to make this change, even if we decided to not move the location for SearchFactory: cleans up a bit of the SearchFactoryIntegrator/SearchFactory/SearchFactoryImplementor/SearchFactoryImplementorWithShareableState hierarchy which we have grown over time to accomodate some of these integration needs. > >>> org.hibernate.search.SearchException -> >>> org.hibernate.search.exception.SearchException >> >> Maybe making the package plural? We should add variations for specific >> failures. > > +1 I was about to suggest that as well ok >>> org.hibernate.search.FullTextFilter -> >>> org.hibernate.search.filter.FullTextFilter >> >> "filtering" ? "filters" ? >> Not too fond of the singular term as it's not very explicit (yes I >> know it's an existing one). > > I think I would stick with filter. I guess it comes down to how disruptive we > want to be ok >>> org.hibernate.search.Version -> >>> org.hibernate.search.engine.Version >> >> +1 > > :-) ok :-) Hoping I didn't miss some crucial point, I think we actually have a reasonable plan. Curious now to hear Emmanuel's POV. Downsides summary: we'd be breaking significant public APIs of both Hibernate Search and Infinispan Query, and also some SPIs :-/ The only reason I'm even thinking of this is that actually some of these changes look like reasonable even out of the OSGi requirements scope :-) Sanne _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
