[jira] [Updated] (SLING-11470) Revert discovery.base impl separation, bump major package version instead

["Stefan Egli (Jira)" <ji...@apache.org>]

     [ 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)