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

[ResourceBundle][proposal/idea]Doctrine driver factory #1087

Merged
merged 1 commit into from
Feb 21, 2014

Conversation

arnolanglade
Copy link
Contributor

I created a factory for the doctrine driver (defined in the dependencyInjector directory). Before this PR, a factory was 'simulated' in the SyliusResourceExtension. The driver used in BaseExtension was hard coded too. The folder Factory was renamed to Driver because it was not really factories IMO. Spec are added too for each Driver.

@Sylius, is it a good idea (I hope!)?

{
/** @var ContainerBuilder $container */
protected $container;
/** @param string $prefix */
Copy link
Contributor

Choose a reason for hiding this comment

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

/**
* @var
*/

@arnolanglade
Copy link
Contributor Author

@jjanvier done!

public function __construct($driver, $className)
{
parent::__construct(sprintf(
'Driver "%s" is unsupported by %s.',
Copy link
Contributor

Choose a reason for hiding this comment

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

is not supported

@arnolanglade
Copy link
Contributor Author

@jjanvier, BaseExtension only use doctrineORM driver (I did not change the default behaviour). I fix the SyliusResourceExtension.

@arnolanglade
Copy link
Contributor Author

Rebased. BaseExtension use the configured driver.

@jjanvier
Copy link
Contributor

Elegant solution, much cleaner 👍

@pjedrzejewski
Copy link
Member

Very nice Arnaud! 👍 Thank you!

pjedrzejewski pushed a commit that referenced this pull request Feb 21, 2014
[ResourceBundle][proposal/idea]Doctrine driver factory
@pjedrzejewski pjedrzejewski merged commit 1675c4c into Sylius:master Feb 21, 2014
@stloyd stloyd added this to the 1.0.0-BETA1 milestone Feb 21, 2014
@jjanvier
Copy link
Contributor

@Arn0d welcome bro. We'll have to celebrate this 🍻

@arnolanglade
Copy link
Contributor Author

@jjanvier yes, next friday... Cuitas les bananas!

@arnolanglade arnolanglade deleted the driver-factory branch February 26, 2014 14:19
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
[ResourceBundle][proposal/idea]Doctrine driver factory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants