I tend to agree that there is not that much value in moving this to the
api. I don't see any use cases for users of Sling's resource api to also
use this class.
Apart from the reasons mentioned, ResourceType talks about versioning -
a concept we currently don't have in the api.
In addition - at least as-is - it creates two more dependencies for the
api: the osgi framework for the Version class and commons lang3 (that
one can be easily removed).
Regards
Carsten
Am 12.04.2021 um 18:32 schrieb Radu Cotescu:
Hi Julian,
The reason for pushing back on this is that ResourceType [1] is more of a
utility class, rather than an API one. It’s a final class, used to:
Generate require and provide capabilities with the help of the
scriptingbundle-maven-plugin
Set up the servlets for each provided capability in
org.apache.sling.servlets.resolver
Granted, we can move it into a dedicated package in o.a.s.api, but I really
don’t see the value. Moving it will also make o.a.s.scripting.spi (providing
the rest of the API needed to support bundled scripts) depend on o.a.s.api. Is
it really worth it?
Regards,
Radu
[1] -
https://github.com/apache/sling-org-apache-sling-scripting-spi/blob/e14f6c23df591646d01fb3aaa185e9360cadcaa0/src/main/java/org/apache/sling/scripting/spi/bundle/ResourceType.java#L41
On 12 Apr 2021, at 16:57, Julian Sedding <[email protected]> wrote:
Hi *
I agree with Olli that moving ResourceType later will not happen,
because it causes too much disruption. I also agree that it is
generally useful and not inherently linked to scripting.
Why can't we just move it now, before we make our lives harder? If
o.a.s.scripting.spi should not depend on o.a.s.api, then it would
still be possible to embed/inline a self-contained class ResourceType.
Regards
Julian
On Thu, Apr 8, 2021 at 10:56 PM Oliver Lietz <[email protected]> wrote:
On Thursday, 8 April 2021 15:48:27 CEST Radu Cotescu wrote:
Hi Olli,
Hi Radu,
On 1 Apr 2021, at 21:26, Oliver Lietz <[email protected]> wrote:
I have a use case for ResourceType not related to Scripting and would like
to see it in Sling API (see SLING-9999 for reasons) where it fits well.
https://github.com/apache/sling-org-apache-sling-scripting-spi/blob/master
/
<https://github.com/apache/sling-org-apache-sling-scripting-spi/blob/mast
er/> src/main/java/org/apache/sling/scripting/spi/bundle/ResourceType.java
WDYT?
I’ve already replied to your concern at [0], but I don’t mind repeating what
I said:
“If we really need to use the ResourceType class somewhere else, we can move
it to the o.a.s.api bundle when the time comes. We can then deprecate the
one from o.a.s.scripting.spi and remove it once there will be other
incompatible changes added by the other interfaces. Until then, I really
don't see the need to spread 5 interfaces over 2 bundles and increase the
dependency footprint for the current consumers.”
And still it does not make sense nor is it consistent.
My initial proposal was to have the Bundle API in bundle o.a.s.scripting.api
(no additional bundle) but you rejected to not depend on Scripting API.
The o.a.s.scripting.spi bundle is fine but the class (not an interface)
ResourceType should be in Sling API from the beginning:
- it is useful in different contexts not related to Scripting
- better visibility in Sling API
- most modules depend on Sling API anyways
- close to Resource classes (o.a.s.api.resource), similar helpers (e.g.
o.a.s.api.resource.path)
- BundledRenderUnitCapability depends on ResourceType, moving/changing it
later is a major change (we try to prevent such breaking changes in Sling)
- it is no effort to do it now, we need a new release of Sling API anyways
Unless you have something already committed to a repository from which you
plan to cut a release soon I'd prefer to go ahead without it for now.
So for now would mean never, see above.
Regards,
O.
Cheers,
Radu
[0] -
https://issues.apache.org/jira/browse/SLING-9999?focusedCommentId=17295320&
page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#commen
t-17295320
--
--
Carsten Ziegeler
Adobe Research Switzerland
[email protected]