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.

Sign up for GitHub

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

Jump to bottom

EZP-30462: Change ez.fieldFormMapper tag to ezplatform.field_type.form_mapper #292

Merged
merged 3 commits into from
Jun 19, 2019
Merged

EZP-30462: Change ez.fieldFormMapper tag to ezplatform.field_type.form_mapper #292

merged 3 commits into from
Jun 19, 2019

Conversation

mikadamczyk
Copy link
Contributor

@mikadamczyk mikadamczyk commented May 21, 2019 •
edited
Loading

Jira: https://jira.ez.no/browse/EZP-30462

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

Relates to:
ezsystems/ezpublish-kernel#2653
ezsystems/ezplatform-matrix-fieldtype#15

ez.fieldFormMapper.value => ezplatform.field_type.form_mapper.value
ez.fieldFormMapper.definition => ezplatform.field_type.form_mapper.definition

alongosz
alongosz requested changes Jun 4, 2019
View reviewed changes
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

Looks good, I'm a bit lost on version numbering though.

bundle/DependencyInjection/Compiler/FieldTypeFormMapperDispatcherPass.php Outdated
$ezplatformFieldFormMapperDefinitionTags = $container->findTaggedServiceIds(self::EZPLATFORM_FIELD_TYPE_FORM_MAPPER_DEFINITION);

foreach ($ezFieldFormMapperValueTags as $ezFieldFormMapperValueTag) {
@trigger_error('`ez.fieldFormMapper.value` service tag is deprecated and will be removed in version 9. Please use `ezplatform.field_type.form_mapper.value` instead.', E_USER_DEPRECATED);
Copy link
Member

@alongosz alongosz Jun 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In version 9? That's too long :)
As this lands in container log, maybe let's use Product name: eZ Platform 4.0.
CS: Please reformat it into multiple lines so it's easier to review, as you can see 2/3 of reviewers missed that. Since you have constant for that tag, you could use sprintf for instance.

bundle/DependencyInjection/Compiler/FieldTypeFormMapperDispatcherPass.php Outdated
}

foreach ($ezFieldFormMapperDefinitionTags as $ezFieldFormMapperValueTag) {
@trigger_error('`ez.fieldFormMapper.definition` service tag is deprecated and will be removed in version 9. Please use `ezplatform.field_type.form_mapper.definition` instead.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

@mikadamczyk mikadamczyk self-assigned this Jun 6, 2019
@mikadamczyk mikadamczyk requested a review from alongosz June 6, 2019 12:26
@mikadamczyk
Copy link
Contributor Author

@alongosz fixed and rebased

@mikadamczyk mikadamczyk mentioned this pull request Jun 6, 2019
Merged
4 tasks
@micszo micszo self-assigned this Jun 18, 2019
@micszo
Copy link
Member

micszo commented Jun 18, 2019

Have you guys analysed the Travis status already? 🙂
Job one fails with:
PHP Fatal error: Declaration of EzSystems\RepositoryFormsBundle\Tests\DependencyInjection\Compiler\FieldTypeFormMapperDispatcherPassTest::registerCompilerPass(Symfony\Component\DependencyInjection\ContainerBuilder $container) must be compatible with Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase::registerCompilerPass(Symfony\Component\DependencyInjection\ContainerBuilder $container): void in /home/travis/build/ezsystems/repository-forms/tests/RepositoryFormsBundle/DependencyInjection/Compiler/FieldTypeFormMapperDispatcherPassTest.php on line 18

Job two fails with:
PHP Fatal error: require(): Failed opening required '/var/www/vendor/autoload.php' (include_path='.:/usr/local/lib/php') in /var/www/bin/console on line 12
Script @php bin/console ezplatform:install clean handling the ezplatform-install event returned with error code 255

@micszo
Copy link
Member

micszo commented Jun 18, 2019

Is it the required PR in kernel or something else?

@mikadamczyk mikadamczyk mentioned this pull request Jun 18, 2019
Merged
2 tasks
micszo
micszo approved these changes Jun 19, 2019
View reviewed changes
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 merged commit 9f8b464 into ezsystems:master Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamwojs adamwojs adamwojs approved these changes

@Nattfarinn Nattfarinn Nattfarinn approved these changes

@pawbuj pawbuj pawbuj approved these changes

@alongosz alongosz alongosz approved these changes

@micszo micszo micszo approved these changes

@webhdx webhdx Awaiting requested review from webhdx

@ViniTou ViniTou Awaiting requested review from ViniTou

Assignees

@mikadamczyk mikadamczyk

Labels
Feature QA approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mikadamczyk @micszo @adamwojs @Nattfarinn @pawbuj @alongosz