Create an interface for TranslationWriter #23665
Conversation
| /** | ||
| * Writes translation from the catalogue according to the selected format. | ||
| * | ||
| * @param MessageCatalogue $catalogue The message catalogue to dump |
There was a problem hiding this comment.
These PHPdoc comments use the "dump" verb, but the interface and method names use "write". Should they be consistent?
| * | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| public function writeTranslations(MessageCatalogue $catalogue, $format, $options = array()); |
There was a problem hiding this comment.
I guess we can't change this now, but according to Symfony naming conventions, "When an object has a "main" many relation with related "things" (objects, parameters, ...), the method names are normalized". So, this method should probably be called write() instead of writeTranslations().
There was a problem hiding this comment.
Sure we can. It is a new interface =)
|
I made the new interface follow the Symfony naming conventions. This means that I deprecated |
| @@ -18,7 +18,7 @@ | |||
|
|
|||
| class TranslationWriterTest extends TestCase | |||
| { | |||
| public function testWriteTranslations() | |||
There was a problem hiding this comment.
The legacy test should be kept with @group legacy and @expectedDeprecation [msg] phpdoc annotations.
|
@Nyholm We still need to add a test for the new |
|
Thank you @chalasr. I've added a test now. |
UPGRADE-3.4.md
Outdated
| Translation | ||
| ----------- | ||
|
|
||
| * Deprecated `Symfony\Component\Translation\Writer\TranslationWriter::writeTranslations` |
There was a problem hiding this comment.
... has been deprecated and will be removed in 4.0, use ... instead
UPGRADE-4.0.md
Outdated
| @@ -506,6 +506,8 @@ Translation | |||
| ----------- | |||
|
|
|||
| * Removed the backup feature from the file dumper classes. | |||
|
|
|||
| * Deprecated `Symfony\Component\Translation\Writer\TranslationWriter::writeTranslations` function, use `Symfony\Component\Translation\Writer\TranslationWriter::write` instead. | |||
| */ | ||
| public function writeTranslations(MessageCatalogue $catalogue, $format, $options = array()) | ||
| { | ||
| @trigger_error('Method writeTranslations() is deprecated. Use write() instead.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
@trigger_error(sprintf('Method %s() is deprecated since version 3.4 and will be removed in 4.0. Use write() instead.', __METHOD__), E_USER_DEPRECATED);
| @@ -88,4 +88,19 @@ public function writeTranslations(MessageCatalogue $catalogue, $format, $options | |||
| // save | |||
| $dumper->dump($catalogue, $options); | |||
| } | |||
|
|
|||
| /** | |||
| * @param MessageCatalogue $catalogue The message catalogue to write | |||
There was a problem hiding this comment.
the method description should be kept
|
Thank you for the review |
| @@ -18,6 +18,10 @@ | |||
|
|
|||
| class TranslationWriterTest extends TestCase | |||
| { | |||
| /** | |||
| * @group legacy | |||
| * @expectedDeprecation Method writeTranslations() is deprecated. Use write() instead. | |||
d056c0e to
faf7c2b
Compare
|
Fabbot complains :) |
|
Thank you. I've update the PR |
|
Is it anything I can do to get this PR merged? |
|
Thank you @Nyholm. |
This PR was squashed before being merged into the 3.4 branch (closes #23665). Discussion ---------- Create an interface for TranslationWriter | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | ~~~no~~~ yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | We should provide an interface for the TranslationWriter. This allows other libraries (php-translation) that depend on the Translation component provide different implementations. Commits ------- c25eafa Create an interface for TranslationWriter
|
Thank you for merging. |
We should provide an interface for the TranslationWriter. This allows other libraries (php-translation) that depend on the Translation component provide different implementations.