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

Merge Translation & TranslationBundle into Resource? #3750

Closed
pjedrzejewski opened this issue Dec 18, 2015 · 3 comments · Fixed by #4256
Closed

Merge Translation & TranslationBundle into Resource? #3750

pjedrzejewski opened this issue Dec 18, 2015 · 3 comments · Fixed by #4256
Labels
DX Issues and PRs aimed at improving Developer eXperience. RFC Discussions about potential changes or new features.

Comments

@pjedrzejewski
Copy link
Member

Hey folks!

I would like to propose merging Translation and TranslationBundle into Resource and ResourceBundle respectively.

Caveats:

  • You will still be able to creates resources which are not translatable. Just implement TranslatableInterface or not and configure translation node or not.

Rationale:

Pros:

  • Removes overhead of having 4 packages instead of 2;
  • Removes overhead of decoupling configuration and its processing in ResourceBundle;
  • We can use TranslatableInterface and other interfaces safely everywhere to check if something supports translations, now we need to use string and check if bundle exists etc. - really annoying;
  • Translation & TranslationBundle without ResourceBundle do not make much sense, is anyone actually using them without Resource?

Cons:

  • I don't see any right now, but opening this RFC to find out. :)

I know that in perfect scenario we create an extensions system for ResourceBundle and every extension could add its own configuration node to every resource and also process metadata in its own way etc. and I'd love to do that but at this point we don't have time and I really don't see a reason to have it decoupled. It only adds overhead.

If someone thinks of using Translation from Sylius standalone, there are already bundles/components which have similar level of functionality.

@pjedrzejewski pjedrzejewski added RFC Discussions about potential changes or new features. DX Issues and PRs aimed at improving Developer eXperience. labels Dec 18, 2015
@pjedrzejewski
Copy link
Member Author

Ref. #3748 and #3721.

@gperdomor
Copy link
Contributor

👍

@bendavies
Copy link
Contributor

+1 sounds good.
sort of related, there is a lot of duplication of the configuration tree structures at the moment. I wonder if they can be made more DRY?

kklecho added a commit to kklecho/Sylius that referenced this issue Sep 2, 2016
Sylius#1595 - Resource bundle missing state machine
Sylius#3750 - Merge Translation & TranslationBundle into Resource
Sylius#3778 - Merge Translation into Resource + some clean ups 
Sylius#4633 - dependency on a non-existent service
kklecho added a commit to kklecho/Sylius that referenced this issue Sep 4, 2016
As sugested by patie here: Sylius#1595

 Sylius#1595 - Resource bundle missing state machine
 Sylius#3750 - Merge Translation & TranslationBundle into Resource
 Sylius#3778 - Merge Translation into Resource + some clean ups
 Sylius#4633 - dependency on a non-existent service
gorkalaucirica pushed a commit to gorkalaucirica/Sylius that referenced this issue Sep 8, 2016
As sugested by patie here: Sylius#1595

 Sylius#1595 - Resource bundle missing state machine
 Sylius#3750 - Merge Translation & TranslationBundle into Resource
 Sylius#3778 - Merge Translation into Resource + some clean ups
 Sylius#4633 - dependency on a non-existent service
Niiko pushed a commit to Niiko/Sylius that referenced this issue Sep 9, 2016
As sugested by patie here: Sylius#1595

 Sylius#1595 - Resource bundle missing state machine
 Sylius#3750 - Merge Translation & TranslationBundle into Resource
 Sylius#3778 - Merge Translation into Resource + some clean ups
 Sylius#4633 - dependency on a non-existent service
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. RFC Discussions about potential changes or new features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants