Hi Radu > Am 21.11.2014 um 18:39 schrieb Radu Cotescu <[email protected]>: > > Hi Felix, > > I pushed all the required changes to the forked repo. Things left to do: > > * check what we actually need to export for preventing class loading > problems
Excellent. I would prefer to really not export most of the RenderUnit stuff incl. BaseRenderUnit and RenderContext. At one point I had a change, where I actually had RenderContext as an interface with a subset of the API, basically just getWriter (does that need to be a StackedWriter after all ?) and a RenderContextImpl which had all the rest but was internal. > * expand JavaDoc > > At this point I think that we can create a patch with our changes and apply > it to trunk. We might want to continue on the fork to find the classes that we really want/need to export. Because this may cause further shuffling of packages etc. Thanks Felix > > Cheers, > Radu > > On Thu, Nov 20, 2014 at 6:53 PM, Felix Meschberger <[email protected]> > wrote: > >> Hi >> >> Thanks for your reply. >> >> For those interested in following, Radu and I are cooperating on these >> changes in my GitHub fork at https://github.com/fmeschbe/sling >> >> >>> Am 19.11.2014 um 22:53 schrieb Radu Cotescu <[email protected]>: >>> >>> Hi Felix, >>> >>> The API reorganisation looks good to me. >>> >>> On Wed, Nov 19, 2014 at 10:49 PM, Felix Meschberger <[email protected]> >>> wrote: >>> >>>> Apart from splitting the base .api package, this are further >> discussions: >>>> >>>> * Exceptions are forming a hierarchy with SightlyEngineException being >> the >>>> root and itself extending from SlingException >>>> >>> >>> Sure, it makes sense. >> >> Done in the fork. The „root“ exception for Sightly is not SightlyException >> (SightlyEngineException renamed to SightlyException). >> >>> >>> >>>> * I wonder, whether we should really drop the *Component abstract >> classes >>>> (RuntimeExtensionComponent and UseProviderComponent). I don’t think they >>>> provide real value at all but have an activate method, which they >> expect to >>>> be called and which they expect to be a DS component annotated with the >>>> Felix annotations. I think this is brittle. >>>> >>> >>> The abstract classes were written specifically for that activate method, >> in >>> order to avoid writing those lines of code for every implementation. >> >> We are working on removing these abstract classes by leveraging service >> properties for >> >> * UseProvider ranking (service.ranking) >> * ExtensionProvider naming >> >>> >>> >>> >>>> * UseProvider defines an ordering method and declares itself being >>>> Comparable. I think this is wrong. The UseProviders should be sorted >> using >>>> regular OSGi service ranking. >>>> >>> >>> If we make the service.ranking property configurable (metatype = true) I >>> guess we could use the OSGi ranking. Theoretically the Use providers can >> be >>> configured to be called in a pre-determined order, depending on how the >>> Sightly projects from an instance make use of the Use-API (Java Use-API >>> objects deployed in bundles / Java Use objects near the component script >> in >>> the repo / JS Use-API, etc.). Having the Use providers called in the most >>> favourable way can increase performance for Sightly applications. >> >> Yes, the service.ranking property can be added to metatype for >> configurable ranking. >> >>> >>> >>>> * I wonder, whether we really have to have all this API exported at all: >>>> Some API I don’t see implemented at all (e.g. BaseRenderUnit — or is >> that >>>> the base class for the Java classes generated from the Sightly >> templates ?) >>>> >>> >>> The BaseRenderUnit is indeed the base class for classes generated from >>> Sightly scripts, which is why we need to export the other classes as >> well. >>> In this regard Sightly is no different from the JSP engine. >> >> Ok. We’d have to test this more. But I suspect we don’t really need to >> export the BaseRenderUnit abstract class. >> >> IIRC the reason for exporting the base JSP servlet is that other classes >> in the same package must be exported. >> >>> >>> >>>> * Last but not least (actually most important of all): There is close to >>>> no JavaDoc. Before cutting a release of this bundle, JavaDoc must be >>>> provided for the API. For example I am not sure, what the Record or the >>>> ObjectModel interfaces are really used for. >>>> >>> >>> It's on my TODO list. >> >> Cool. Thanks. >> >> Regards >> Felix >>
