Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-1694: Implemented Symfony Service Tag compatibility layer #12

Merged
merged 6 commits into from
Jan 17, 2022

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Dec 15, 2021

Question Answer
JIRA issue IBX-1694
Type feature
Target Ibexa version v4.0
BC breaks no

This PR adds compatibility layer for the legacy eZ Platform Symfony Service Container Tag names, defining the mapping between old names and new Ibexa names.

The solution is to find all legacy tags and add another tag with the new name and the same attributes. Legacy tag names are not removed from the definitions due to BC. Even if we ourselves remove old tags, there's no way to know if some 3rd party still relies on them.

  • What needs to be discussed here are the new naming choices made here. Grouping is done by the dots (.) in the new tag names, but groups are related to domain/functionality rather than a package defining the tag. If there's a certain pattern associated with a tag (e.g.: visitor, handler), it is placed at the end of a name.

Naming convention

ibexa.<domain>.[adapter].<specifics>.[design_pattern]

where <specifics> may or may not be grouped (aiming not to group) per case basis. If a group can be identified for certain items, it should be used. [] implies optional part.

Previously deprecated tags

The support for Tags that were deprecated in Ibexa 3.3 or earlier is going to be dropped. They're not included in the map. Some leftover usages will be replaced across the codebase with the new name.

  • ezpublish.searchEngineIndexer
  • ezpublish.searchEngine
  • ezpublish.fieldType.externalStorageHandler
  • ezpublish.fieldType.externalStorageHandler.gateway
  • ezpublish.fieldType
  • ezpublish_rest.field_type_processor
  • ezpublish.fieldType.parameterProvider
  • ezpublish.storageEngine.legacy.converter
  • ezpublish.fieldType.indexable
  • ez.fieldFormMapper.definition
  • ez.fieldFormMapper.value

TODO

  • Update mapping with all needed tags
  • Add minimal test coverage

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review

@alongosz alongosz added Feature New feature request Work in progress labels Dec 15, 2021
@alongosz alongosz force-pushed the ibx-1694-svc-tag-compat-layer branch from c6f59ae to 2dd3594 Compare December 15, 2021 16:34
@alongosz alongosz marked this pull request as ready for review December 15, 2021 16:44
@alongosz alongosz requested review from Nattfarinn, ViniTou and a team December 15, 2021 16:44
@konradoboza konradoboza requested a review from a team December 16, 2021 07:28
@alongosz
Copy link
Member Author

@konradoboza #12 (review) resolved via 914caf7. Thanks!

@konradoboza
Copy link
Contributor

Yet to be discussed, but it feels like we should go either with . or _ as a separator in general. Having one specific pattern would speed up the process as end developers won't be bothered with any conventions at all while tagging. Determining what should "go together" in naming is relative, it feels like we would benefit from a rule of thumb here. Despite documenting this change I think it will be easier to just assume, that one rule is reused across the system and it's safe to follow it whenever service tags are applied.

Copy link
Contributor

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention is still a little bit blurry to me, but the general naming looks fine. Few concerns:

@alongosz alongosz added the Doc needed The changes require some documentation label Dec 23, 2021
@alongosz
Copy link
Member Author

Update: fc42c63: removed ezsystems.platformui.application_config_provider from the map as it seems to be an unused leftover from v1.x. There are tagged services (#12), but tag itself is not processed anywhere.

@alongosz alongosz merged commit 5e06de7 into main Jan 17, 2022
@alongosz alongosz deleted the ibx-1694-svc-tag-compat-layer branch January 17, 2022 10:36
@DominikaK DominikaK removed the Doc needed The changes require some documentation label Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature request Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants