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

Remove required config #86

Closed
wants to merge 3 commits into from
Closed

Remove required config #86

wants to merge 3 commits into from

Conversation

blanchonvincent
Copy link
Contributor

I think it's better for try the project.
It's just a proposition. Let me know.

{
if (null === $configuration) {
Copy link
Owner

Choose a reason for hiding this comment

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

$this->configuration = $configuration ?: new Configuration();

@Ocramius
Copy link
Owner

Looks good!

I was thinking of implementing a small ProxyManager\ProxyManager class that eases use of the factories at some point in time (acts as a micro service container for direct consumers, but with no additional dependencies)

*
* @covers \ProxyManager\Factory\LazyLoadingGhostFactory::__construct
*/
public function testWithOptionnalFactory()
Copy link
Owner

Choose a reason for hiding this comment

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

Optional

@blanchonvincent
Copy link
Contributor Author

@Ocramius done, than you for the review

@Ocramius Ocramius closed this in f4f8ed9 Sep 13, 2013
@Ocramius
Copy link
Owner

@blanchonvincent tested locally, merged, thanks!

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

Successfully merging this pull request may close these issues.

2 participants