[
https://issues.apache.org/jira/browse/SLING-11470?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Stefan Egli updated SLING-11470:
--------------------------------
Description:
This is a follow-up of SLING-11355 ([discovery-base
PR#7|https://github.com/apache/sling-org-apache-sling-discovery-base/pull/7])
As part of that PR impl classes of announcement and ping packages got moved to
impl subpackages to have better separation. Also, the package version bump was
suppressed.
As now noticed by [~mreutegg], there is an issue with this change (that blocks
[discovery-oak
PR#7|https://github.com/apache/sling-org-apache-sling-discovery-oak/pull/7] ) :
the {{CachedAnnouncement}} is part if the announcement package's API but was
now made private by moving it to impl.
Several options how to fix this probably, listing two of them:
# keep the impl class separated. But fix {{CachedAnnouncement}} by placing it
back to the public package. This would require the class to be split into an
interface/implementation pair to avoid making registerPing public.
Additionally, continue the impl separation for the ping package by the other 2
remaining implementation classes also to impl : {{TopologyConnectorServlet}}
and {{TopologyRequestValidator}} (with corresponding adjustments in tests).
# go back to the original, non separated way (even though this was not best
practice).
Also:
* in both cases I would actually argue (a bit late) to not overrule the
baseline check and actually do the major version bumps. In hindsight seems more
appropriate, as it would ensure downstream users do the required upgrade.
So, I would vote for option 2 + package bumps, as these are fewer changes and
the discovery.base package is mostly really only used by discovery.oak these
days, so I don't see a strong need for beautifying and introducing impl
separation.
[~apelluru], [~kwin], [~rombert], wdyt?
was:
This is a follow-up of SLING-11355 ([discovery-base
PR#7|https://github.com/apache/sling-org-apache-sling-discovery-base/pull/7])
As part of that PR impl classes of announcement and ping packages got moved to
impl subpackages to have better separation. Also, the package version bump was
suppressed.
As now noticed by [~mreutegg], there is an issue with this change (that blocks
[discovery-oak
PR#7|https://github.com/apache/sling-org-apache-sling-discovery-oak/pull/7] ) :
the {{CachedAnnouncement}} is part if the announcement package's API but was
now made private by moving it to impl.
Several options how to fix this probably, listing two of them:
# keep the impl class separated. But fix {{CachedAnnouncement}} by placing it
back to the public package. This would require the class to be split into an
interface/implementation pair to avoid making registerPing public.
Additionally, continue the impl separation for the ping package by the other 2
remaining implementation classes also to impl : {{TopologyConnectorServlet}}
and {{TopologyRequestValidator}} (with corresponding adjustments in tests).
# go back to the original, non separated way (even though this was not best
practice).
Also:
* in both cases I would actually argue (a bit late) to not overrule the
baseline check and actually go the major version bumps. In hindsight seems more
appropriate, as it would ensure downstream users do the required upgrade.
So, I would vote for option 2 + package bumps, as these are fewer changes and
the discovery.base package is mostly really only used by discovery.oak these
days, so I don't see a strong need for beautifying and introducing impl
separation.
[~apelluru], [~kwin], [~rombert], wdyt?
> Revert discovery.base impl separation, bump major package version instead
> -------------------------------------------------------------------------
>
> Key: SLING-11470
> URL: https://issues.apache.org/jira/browse/SLING-11470
> Project: Sling
> Issue Type: Task
> Components: Discovery
> Reporter: Stefan Egli
> Assignee: Stefan Egli
> Priority: Major
>
> This is a follow-up of SLING-11355 ([discovery-base
> PR#7|https://github.com/apache/sling-org-apache-sling-discovery-base/pull/7])
> As part of that PR impl classes of announcement and ping packages got moved
> to impl subpackages to have better separation. Also, the package version bump
> was suppressed.
> As now noticed by [~mreutegg], there is an issue with this change (that
> blocks [discovery-oak
> PR#7|https://github.com/apache/sling-org-apache-sling-discovery-oak/pull/7] )
> : the {{CachedAnnouncement}} is part if the announcement package's API but
> was now made private by moving it to impl.
> Several options how to fix this probably, listing two of them:
> # keep the impl class separated. But fix {{CachedAnnouncement}} by placing
> it back to the public package. This would require the class to be split into
> an interface/implementation pair to avoid making registerPing public.
> Additionally, continue the impl separation for the ping package by the other
> 2 remaining implementation classes also to impl :
> {{TopologyConnectorServlet}} and {{TopologyRequestValidator}} (with
> corresponding adjustments in tests).
> # go back to the original, non separated way (even though this was not best
> practice).
> Also:
> * in both cases I would actually argue (a bit late) to not overrule the
> baseline check and actually do the major version bumps. In hindsight seems
> more appropriate, as it would ensure downstream users do the required upgrade.
> So, I would vote for option 2 + package bumps, as these are fewer changes and
> the discovery.base package is mostly really only used by discovery.oak these
> days, so I don't see a strong need for beautifying and introducing impl
> separation.
> [~apelluru], [~kwin], [~rombert], wdyt?
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
