Skip to content

[Validator] Deprecated interface still required for TranslationInterface in Validator #31025

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

Closed

Conversation

snebes
Copy link

@snebes snebes commented Apr 8, 2019

Q A
Branch? 4.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #30591 (related)
License MIT

This PR removes the hard requirement for the LegacyValidatorInterface used by replacing the type-hint with a docBlock typehint for either the non-deprecated or deprecated TranslatorInterface.

Also, updated the test to use the new TranslatorInterface contract.

@snebes snebes changed the title [Validator] Removed legacy interfaces [Validator] Deprecated interface still required for TranslationInterface in Validator Apr 8, 2019
@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Apr 8, 2019
@@ -256,8 +256,11 @@ public function setConstraintValidatorFactory(ConstraintValidatorFactoryInterfac
/**
* {@inheritdoc}
*/
public function setTranslator(LegacyTranslatorInterface $translator)
public function setTranslator($translator)
Copy link
Member

Choose a reason for hiding this comment

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

Changing this is a BC break - but that might be OK as maybe nobody extends this class.
If not, we need to rethink the BC layer.

@snebes snebes force-pushed the validator-legacy-translator-interface-update branch from d247e67 to 4301a56 Compare April 8, 2019 21:10
@snebes snebes closed this Apr 8, 2019
@snebes snebes deleted the validator-legacy-translator-interface-update branch April 8, 2019 21:27
@snebes snebes restored the validator-legacy-translator-interface-update branch April 8, 2019 21:27
@snebes
Copy link
Author

snebes commented Apr 8, 2019

Accidentally closed this one

@snebes snebes reopened this Apr 8, 2019
@nicolas-grekas
Copy link
Member

Thinking twice, this change is wrong - as in "BC break".
What needs to be done instead is wrapping the translator passed to setTranslator in a LegacyTranslatorProxy.

@snebes snebes closed this Apr 10, 2019
@snebes snebes deleted the validator-legacy-translator-interface-update branch April 10, 2019 15:55
@stof
Copy link
Member

stof commented Apr 12, 2019

but then, we need to update FrameworkBundle to perform this wrapping. Otherwise, we force the translator service to implement the old interface.

@stof
Copy link
Member

stof commented Apr 12, 2019

btw, this also means we don't have a migration path on the ValidatorBuilder for now.

@nicolas-grekas
Copy link
Member

btw, this also means we don't have a migration path on the ValidatorBuilder for now.

we have one: the class is made final

fabpot pushed a commit that referenced this pull request Apr 17, 2019
…or with LegacyTranslatorProxy (nicolas-grekas)

This PR was merged into the 4.2 branch.

Discussion
----------

[FrameworkBundle] decorate the ValidatorBuilder's translator with LegacyTranslatorProxy

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31092, #31025
| License       | MIT
| Doc PR        | -

This allows defining a translator that implements only the new interface and use it with ValidatorBuilder.

ping @dvdknaap, @snebes since you were affected.

Commits
-------

a12656e [FrameworkBundle] decorate the ValidatorBuilder's translator with LegacyTranslatorProxy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants