Conversation
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function trans(?string $id, array $parameters = [], ?string $domain = null, ?string $locale = null): string |
There was a problem hiding this comment.
I would use a multiline signature here
|
@jonasdekeukelaere Can you please fix the conficts? @dsoriano I think this is the solution we need to fix the Symfony8 "final" bug |
|
@bartmcleod there is 2 methods to fix the issue:
This one seems to be a mix between the two. It creates a standalone Translator but simulate a decorator by creating a new instance of the Symfony Translator (line 74). I don't think this is the right way. |
|
@dsoriano Well, it wraps the Symfony translator. I can see where you are coming from, but as I am not a hard core Symfony guy, I have a hard time being religious about it. If you can manage the decorator solution, that sounds elegant. We can close this one if you provide it and if it actually looks better. |
|
@bartmcleod I'm on it 😉 |
|
@jonasdekeukelaere Thank you for your contribution. Your solution might not follow all best practices that are expected from us, but I believe it will work. Because currently, the bundle is broken for PHP 8, I think it is best that we merge your solution until someone improves it. That will however require you to fix the conflicts, please. |
|
@jonasdekeukelaere Even without solving the conflicts, checking out your PR and trying for example |
|
@bartmcleod My PR will be ready in a few days. Actually the code is OK, the tests too. I need to improve the compatibility with the current config to avoid BC breaks, improve the tests and try it on a real project to ensure everything is ok. Probably by the end of the week or the start of the next. |
|
Superseded by #504 where I fixed the conflicts and confirmed it now works with Symfony 8 |
Some fixes for Symfony 8 (with the help op copilot)
Fix for #500