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

[WIP] Module support #4

Merged
merged 1 commit into from
Feb 4, 2014
Merged

[WIP] Module support #4

merged 1 commit into from
Feb 4, 2014

Conversation

Ocramius
Copy link
Contributor

@Ocramius Ocramius commented Feb 4, 2014

The idea is simple:

  • composer require "mnapoli/php-di-zf2:1.0.*"
  • Add DI\ZendFramework2\Module to application.config.php under modules
  • Run application
  • ...
  • profit?

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

Ping @juriansluiman

@mnapoli
Copy link
Member

mnapoli commented Feb 4, 2014

Awesome thanks! I guess I will remove the current files, they won't be needed (and I'll release a new version anyway).

mnapoli added a commit that referenced this pull request Feb 4, 2014
Integration to ZF2 using an abstract factory
@mnapoli mnapoli merged commit c7a35cc into PHP-DI:master Feb 4, 2014
@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

Urgh, this was WIP =\

@mnapoli
Copy link
Member

mnapoli commented Feb 4, 2014

Ha :) Was there anything not good?

I checked the documentation (and have to admit I just finally got how abstract factories fit in the service manager) and it seems good? I wanted to try to add some tests.

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

@mnapoli the idea is that any service not defined by ZF2 explicitly goes back to the abstract factory. I wrote all this on top of my head, so it may not work, but it's fairly simple.

@mnapoli
Copy link
Member

mnapoli commented Feb 4, 2014

the idea is that any service not defined by ZF2 explicitly goes back to the abstract factory

it falls back to the abstract factory is that what you mean?

I'll work on some tests, that will help me learn about ZF's service manager too

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

it falls back to the abstract factory is that what you mean?

correct

As for the tests, you can start from https://github.com/doctrine/DoctrineORMModule/blob/31a0a0aefd424ca2eaecc0303c4bb0d959b1f2c0/tests/Bootstrap.php

@juriansluiman
Copy link

LOL

Yeah, this looks WIP and there might be some things to clear out. Just looking at the code, this should work fine. A couple of other things though:

  1. Cleanup of old controller classes
  2. The factory of DI\Container as a service should be registered as a factory itself, not inside a closure.
  3. I don't like the Module.php to be hidden :p

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

Cleanup of old controller classes

Totally agree

The factory of DI\Container as a service should be registered as a factory itself, not inside a closure.

Hmm, I don't see a closure? But yes, DI\Container does not exist as a service AFAIK...

I don't like the Module.php to be hidden :p

You can beat me to death, I will never like this convention of ZF2 :P

@juriansluiman
Copy link

Hmm, I don't see a closure? But yes, DI\Container does not exist as a service AFAIK...

The docs, third code block :)

You can beat me to death, I will never like this convention of ZF2 :P

At least you're on par the Module.php at the root is the convention ;)

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

The docs, third code block :)

Ah, yes, these definition should come from the module, not be up to the user

At least you're on par the Module.php at the root is the convention ;)

I don't like conventions :P
Also, this package assumes composer (I suppose), so I see little benefit in having Module at its root, especially given the nested namespace...

@mnapoli
Copy link
Member

mnapoli commented Feb 4, 2014

Sorry guys for having closed this too fast, I was too enthusiastic ;)

So I cleaned up the controllers, added some tests (yay now I have a better understanding of how ZF2 works) and updated the documentation. Thx for the link @Ocramius btw, it was very useful to get started.

Let me know if the documentation is correct, especially the configuration the user should add:

    // ...
    'modules' => array(
        'DI\ZendFramework2',
    ),
    // ...
    'service_manager' => array(
        // ...
        'factories' => array(
            'DI\Container' => function () {
                $builder = new DI\ContainerBuilder();
                // Configure your container here
                return $builder->build();
            },
        ),
    ),

The factory of DI\Container as a service should be registered as a factory itself, not inside a closure.

Ah, yes, these definition should come from the module, not be up to the user

It is the user that will configure his PHP-DI container though? The module provided by this package can't know what kind of configuration the user will want?

@Ocramius
Copy link
Contributor Author

Ocramius commented Feb 4, 2014

@mnapoli in theory, that closure using the $builder should be responsible for passing in configuration.

Configuration can be fetched via $serviceLocator->get('config') in that closure.

Consider also that the closure should be moved to a factory class

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.

None yet

3 participants