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

set alias default translation strategy #3572

Closed
wants to merge 1 commit into from
Closed

set alias default translation strategy #3572

wants to merge 1 commit into from

Conversation

andrey1s
Copy link

@andrey1s andrey1s commented Feb 3, 2016

No description provided.

@OskarStark
Copy link
Member

ping @rande

@greg0ire
Copy link
Contributor

greg0ire commented Feb 6, 2016

Would fix #2419 very simply :) 👍

@OskarStark
Copy link
Member

@andrey1s i would like to merge your PR, but could you please add a test that the configuration is processed correctly?

Thank you!

@OskarStark OskarStark self-assigned this Feb 8, 2016
@andrey1s
Copy link
Author

andrey1s commented Feb 8, 2016

@OskarStark I don't use the Sonata configuration, only the configuration of the Symfony 2. and create alias for the default strategy. I think the configuration for the Sonata this could be another PR if needed.

@greg0ire
Copy link
Contributor

greg0ire commented Feb 8, 2016

There is a problem with this though : you cannot associate a strategy with a given bundle. So, if you're using SonataUserBundle, changing the strategy will break all the translations and you will have to translate every string from this bundle again. See #3319

@andrey1s
Copy link
Author

andrey1s commented Feb 8, 2016

@greg0ire this PR not changing the default strategy. And of course if the developer changes the strategy then of course he needs to translate.

@greg0ire
Copy link
Contributor

greg0ire commented Feb 8, 2016

if the developer changes the strategy then of course he needs to translate

Yes, it's the problem I'm warning you about. It would be better to be able to change the default strategy for a set of bundles only.

@greg0ire
Copy link
Contributor

greg0ire commented Feb 8, 2016

But maybe this could be merged anyway, with a warning about this problem in the docs? It would be useful for people who only use the Admin Bundle I guess…


services:
sonata.admin.label.strategy.default:
alias: sonata.admin.label.strategy.underscore
Copy link
Member

Choose a reason for hiding this comment

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

Please add a warning that was mentioned by @greg0ire

@core23 core23 added the minor label Apr 21, 2016
@soullivaneuh
Copy link
Member

According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning.

If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch.

So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one).

Regards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants