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

[RFC] Reduce code duplication in DI #439

Merged
merged 1 commit into from
Oct 22, 2013
Merged

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Oct 22, 2013

No description provided.

$this->mapValidationGroupParameters($config['validation_groups'], $container);
}

if ($container->hasParameter('sylius.config.classes')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

App name should be configurable (usefull if you make an app with ressource bundle)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case you can do it on you own if you need it (note that this is replacement for Sylius bundles not custom ones):

list($config) = $this->configure($config, $container, self::CONFIGURE_LOADER | self::CONFIGURE_PARAMETERS | self::CONFIGURE_VALIDATORS);

$container->setParameter('my.own.namespace', array_merge($config['classes'], $container->getParameter('sylius.config.classes')));

@arnolanglade
Copy link
Contributor

👍

@pjedrzejewski
Copy link
Member

Awesome, but I guess you meant to "minimum". :D

@ghost ghost assigned stloyd Oct 22, 2013
@stloyd
Copy link
Contributor Author

stloyd commented Oct 22, 2013

@pjedrzejewski I think it's ready to go.

pjedrzejewski pushed a commit that referenced this pull request Oct 22, 2013
[RFC] Reduce code duplication in DI
@pjedrzejewski pjedrzejewski merged commit ad7351e into Sylius:master Oct 22, 2013
@pjedrzejewski
Copy link
Member

Nice! Thanks a lot Józef! Sometime later we could think about removing the duplication in Configuration classes, but this might be a bit more tricky as there are some differences, to be honest not sure if we can achieve lot there.

@stloyd stloyd deleted the bugfix/cs branch October 22, 2013 11:52
@stloyd
Copy link
Contributor Author

stloyd commented Oct 22, 2013

@pjedrzejewski Can you exclude dir: app/* at Scrutinizer-CI? This would prevent spamming with "issues" in not Sylius based code (i.e. AppKernel, SymfonyRequirements etc.).

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
[RFC] Reduce code duplication in DI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants