Uploaded image for project: 'Sling'
  1. Sling
  2. SLING-11470

Revert discovery.base impl separation, bump major package version instead

    XMLWordPrintableJSON

Details

    • Task
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • None
    • Discovery
    • None

    Description

      This is a follow-up of SLING-11355 (discovery-base PR#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 ) : 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:

      1. 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).
      2. 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?

      Attachments

        Issue Links

          Activity

            People

              stefanegli Stefan Egli
              stefanegli Stefan Egli
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 20m
                  20m