-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add methods used by TranslationUpdateCommand #25860
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
Conversation
``Symfony\Bundle\FrameworkBundle\Command\TranslationUpdateCommand`` uses ``TranslationWriterInterface`` methods ``disableBackup`` and ``getFormats`` not defined by the interface but by the implementation ``Symfony\Component\Translation\Writer\TranslationWriter``.
If you want to make it backwards compatible, you'll probably have to make a second interface and implement that. BC breaks won't me merged until 5.0. In your PR, this is also seen as a new feature. |
I dont want to add more complexity by adding a new interface, can wait for v5.0 since Php is not Java ^^. Also, there is the same problem with: |
One of the 4.x versions will have to add a layer to enable this feature either way. V5 won't be released for almost 2 years ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't break BC in major releases without an upgrade path in a previous minor release.
If one comes with a valid use case for decorating the translation.writer
service, we could add an instanceof check to skip calling the getFormats()
method. The same goes for the calls to Application::getKernel()
, yet it's not super clean but that's under framework-bundle
, we know which implementation is behind it.
👎 Fine as is to me
/** | ||
* Disables dumper backup. | ||
*/ | ||
public function disableBackup(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't have to be interfaced:
* @deprecated since Symfony 4.1, to be removed in 5.0 |
Closing as explained, thanks for proposing. |
…when using "--no-backup" (liarco) This PR was squashed before being merged into the 5.1 branch. Discussion ---------- [FrameworkBundle] Fixing TranslationUpdateCommand failure when using "--no-backup" | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | [5.0.0](https://github.com/symfony/symfony/blob/5.0/src/Symfony/Component/Translation/CHANGELOG.md#500) removed `TranslationWriter::disableBackup()` but `TranslationUpdateCommand` still has `--no-backup` flag. Using that flag throws an error so I think that removing it without deprecation may be the right choice. Thrown error: ``` In TranslationUpdateCommand.php line 287: Attempted to call an undefined method named "disableBackup" of class "Symfony\Component\Translation\Writer\TranslationWriter". ``` Further references to the topic: - #18290 (comment) - #25860 Commits ------- ef24b10 [FrameworkBundle] Fixing TranslationUpdateCommand failure when using "--no-backup"
Symfony\Bundle\FrameworkBundle\Command\TranslationUpdateCommand
usesTranslationWriterInterface
methodsdisableBackup
andgetFormats
not defined by the interface but by the implementationSymfony\Component\Translation\Writer\TranslationWriter
.