-
Notifications
You must be signed in to change notification settings - Fork 265
Fixes loading and importing translations from bundles when translations are in /translations directory #486
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
|
This should also fix #440 |
bartmcleod
left a comment
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.
Thank you for your PR! It looks good to me, just one small modification would make it great, thanks!
| $this->output->writeln(sprintf('<info># %s:</info>', $bundle->getName())); | ||
| $finder = $this->findTranslationsFiles($path, $locales, $domains); | ||
| $this->importTranslationFiles($finder); | ||
| } else { |
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.
Can you maybe replace the else with an early return in the body of the preceding if?
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.
|
@Dukecz Thanks again for your PR. I ran the tests and they fail on some validation of the translation directory. Can you also fix the test please? When the tests pass, it is good to merge. Please also check my comment on the code changes, thanks! |
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.
The changes look good, I'm trying to make the build actions pass, no luck so far, @Dukecz probably easy to fix though, can you run them on your local, or do you need help with that?
|
@bartmcleod tests should finally be fixed |
|
@Dukecz Merged! Thank you for your contribution 🥇 |
…ns are in /translations directory (lexik#486) * fixes loading translations from bundles when translations are in /translations * early return codestyle fix * updated tests * updated tests
…ns are in /translations directory (lexik#486) * fixes loading translations from bundles when translations are in /translations * early return codestyle fix * updated tests * updated tests
…ns are in /translations directory (lexik#486) * fixes loading translations from bundles when translations are in /translations * early return codestyle fix * updated tests * updated tests
…ns are in /translations directory (lexik#486) * fixes loading translations from bundles when translations are in /translations * early return codestyle fix * updated tests * updated tests
No description provided.