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

EZP-30462: Unified Field Type service tag names #2653

Merged
merged 12 commits into from
Jun 19, 2019

Conversation

mikadamczyk
Copy link
Contributor

@mikadamczyk mikadamczyk commented May 21, 2019

Question Answer
JIRA issue EZP-30462
Improvement yes
New feature no
Target version master
BC breaks no
Tests pass yes/no
Doc needed yes

Tags used to register Field Type features in the dependency injection container should be consistent and follow Symfony convention (snake case)

ezpublish.fieldType => ezplatform.field_type
ezpublish.fieldType.indexable => ezplatform.field_type.indexable
ezpublish.storageEngine.legacy.converter => ezplatform.field_type.legacy_storage.converter
ezpublish.fieldType.parameterProvider => ezplatform.field_type.parameter_provider
ezpublish_rest.field_type_processor => ezplatform.field_type.rest.processor

Related PR:
ezsystems/repository-forms#292
ezsystems/ezplatform-matrix-fieldtype#15

QA: We have to make sure that all field types work correctly

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@ViniTou
Copy link
Contributor

ViniTou commented Jun 5, 2019

This must be rebased
@mikadamczyk

@mikadamczyk mikadamczyk changed the base branch from master to remove-nameable-field-type June 6, 2019 07:20
@mikadamczyk
Copy link
Contributor Author

@ViniTou @alongosz it is rebased and ready for review

Copy link
Contributor

@pawbuj pawbuj left a comment

Choose a reason for hiding this comment

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

For twig helpers we choosed ez prefix so I think that we should be consequent and use it also for tags, instead of ezplatform

@mikadamczyk
Copy link
Contributor Author

@alongosz @ViniTou @webhdx @Nattfarinn what do you think?

@Nattfarinn
Copy link
Contributor

Nattfarinn commented Jun 7, 2019

Fair question. It does make sense to rename tags to ez, but on the other hand we'll loose the oportunity to distinguish products in future (to have separate ezplatform and ezplatform-ee tags) in case we want it next major (and as we're thinking about ezpaltform-admin-ui-ee maybe it does make sense as well).

I am 33.33% for ez. and 66.66% for ezplatform. (and 0.01% for capybara. because how awsome they are).

But, to beat next question, I wouldn't go that far to prefix non-FQCN services (and event names in case we keep ezplatform. prefix here) with just ez.. Services in my opinion have to remain product oriented.

@adamwojs
Copy link
Member

adamwojs commented Jun 7, 2019

+1 for the ezplatform prefix.

@alongosz
Copy link
Member

alongosz commented Jun 7, 2019

Correct prefix is ezplatform because we won't use the same tag in a completely different product.
+1 for what @Nattfarinn have just requested.

@mikadamczyk
Copy link
Contributor Author

@Nattfarinn fixed, also @alongosz @pawbuj

@micszo
Copy link
Member

micszo commented Jun 18, 2019

Testing discovered https://jira.ez.no/browse/EZEE-2801.
Update: ticket is closed, it was mistakenly identified to occur on EE master.
It is fixed in ezsystems/ezplatform-matrix-fieldtype#15.

@micszo
Copy link
Member

micszo commented Jun 19, 2019

Btw, stumbled upon the ez star rating issue: https://jira.ez.no/browse/EZP-30677.

@alongosz
Copy link
Member

@micszo resolved conflict with base branch.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

@micszo micszo removed their assignment Jun 19, 2019
@alongosz alongosz changed the title EZP-30462: Configure Field Type features with tags which follows same convention EZP-30462: Unified Field Type service tag names Jun 19, 2019
@alongosz alongosz merged commit e7a552c into ezsystems:master Jun 19, 2019
@emodric
Copy link
Contributor

emodric commented Jul 11, 2019

Why wasn't ezpublish.fieldType.externalStorageHandler tag renamed?

@adamwojs
Copy link
Member

Good catch @emodric, we forgot about this one 😅 I will rename it in follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

8 participants