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

[Resource] Integrated Translations #4256

Merged

Conversation

NoResponseMate
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Related tickets fixes #3750
License MIT
Doc PR -

Based on #3854

@pjedrzejewski pjedrzejewski added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Feb 22, 2016
@@ -1,6 +1,10 @@
CHANGELOG
=========

### v0.17.0

* Integrated TranslationBundle.
Copy link
Contributor

Choose a reason for hiding this comment

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

Merged with TranslationBundle.

@@ -82,7 +82,7 @@ private function addResourcesSection(ArrayNodeDefinition $node)
->scalarNode('model')->defaultValue(Archetype::class)->cannotBeEmpty()->end()
->scalarNode('interface')->defaultValue(ArchetypeInterface::class)->cannotBeEmpty()->end()
->scalarNode('controller')->defaultValue(ResourceController::class)->cannotBeEmpty()->end()
->scalarNode('repository')->defaultValue(TranslatableResourceRepository::class)->cannotBeEmpty()->end()
->scalarNode('repository')->defaultValue(TranslatableRepository::class)->cannotBeEmpty()->end()
Copy link
Member

Choose a reason for hiding this comment

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

Repository should not have default value at all. It is fixed in #3854. This logic should be placed under drivers folder in resource of bundle.

@lchrusciel
Copy link
Member

I think it should be based on #3854, because TranslatableResourceRepository is removed totally there.

@NoResponseMate NoResponseMate force-pushed the move-translation-to-resource branch 6 times, most recently from d85a904 to 3ddb3be Compare March 7, 2016 20:05
@@ -28,7 +29,7 @@
* @author Alexandre Bacco <alexandre.bacco@gmail.com>
* @author Saša Stamenković <umpirsky@gmail.com>
*/
class NumberListener
class NumberListener implements NumberListenerInterface
Copy link
Member

Choose a reason for hiding this comment

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

Listeners should not use interfaces. :)

@pjedrzejewski
Copy link
Member

@NeverResponse Requires rebasing after merge of #3854.

pjedrzejewski pushed a commit that referenced this pull request Mar 9, 2016
@pjedrzejewski pjedrzejewski merged commit 10b547c into Sylius:master Mar 9, 2016
@pjedrzejewski
Copy link
Member

Thanks Jasiek! 👍

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. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge Translation & TranslationBundle into Resource?
5 participants