Skip to content
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

[FrameworkBundle] Fix registration of the bundle path to translation #54063

Merged

Conversation

FlyingDR
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
License MIT

31d7a09 had a small issue resulting in a lack of proper registration of path to translation resources in installed bundles.

The issue is caused by the incomplete update of the code where the newly added $transPaths variable was not updated in all code branches (1, 2, 3, but not 4)

The issue can be reproduced and tested like this:

composer create-project symfony/skeleton test
cd test
composer require easycorp/easyadmin-bundle
php bin/console debug:translation en --domain EasyAdminBundle

It will result in unexpected:

[WARNING] No defined or extracted messages for locale "en" and domain "EasyAdminBundle"

After applying the fix, clearing the cache and running the same command list of messages will be returned properly.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 5, 2024

/cc @yceruto 🙏

@stof
Copy link
Member

stof commented Apr 5, 2024

why do we use 2 variables that must contain the same content ? Wouldn't it be simpler to use a single variable (or maybe to do $transPaths = $dirs when reaching the end of the common initialization instead ?)

Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

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

It can be optimized for sure.

@fabpot
Copy link
Member

fabpot commented Apr 8, 2024

Thank you @FlyingDR.

@fabpot fabpot merged commit 3357441 into symfony:5.4 Apr 8, 2024
9 of 11 checks passed
@FlyingDR FlyingDR deleted the fix-bundle-translation-path-registration branch April 8, 2024 11:12
This was referenced Apr 29, 2024
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.

None yet

7 participants