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

Move EventStoreFactory one namespace level up #69

Closed
codeliner opened this issue Aug 21, 2015 · 12 comments
Closed

Move EventStoreFactory one namespace level up #69

codeliner opened this issue Aug 21, 2015 · 12 comments

Comments

@codeliner
Copy link
Member

Namespace of EventStoreFactory should only be Prooph\EventStore\Service.
We should also think up renaming Service to Factory.
Putting factories under a Service namespace is common practice in ZF2 but maybe this confuses others.

@codeliner codeliner added this to the 5.0 Release milestone Aug 21, 2015
@codeliner
Copy link
Member Author

@prolic What do you think regarding the namespace name?

@prolic
Copy link
Member

prolic commented Aug 21, 2015

Do you want to put all factories in the service namespace or do you want to
get rid of the service namespace?
Am 21.08.2015 08:14 schrieb "Alexander Miertsch" notifications@github.com:

@prolic https://github.com/prolic What do you think regarding the
namespace name?


Reply to this email directly or view it on GitHub
#69 (comment).

@codeliner
Copy link
Member Author

Sorry, my description is confusing (wrote it to early in the morning 😄 )

I would like to put all factories in a Factory namespace:

src
|_ Factory
    |_ Stream
        |_ SingleStreamFactory.php
        |_ ...
    |_ EventStoreFactory.php
|_ ...
|_ EventStore.php

@prolic
Copy link
Member

prolic commented Aug 21, 2015

I am okay with getting rid of the Service\EventStore namespace in favour of only a Service namespace. But I am against the renaming from Service to Factory.
ZF2 uses also the Service namespace for their factories, f.e. Zend\Mvc\Service, see: https://github.com/zendframework/zend-mvc/tree/master/src/Service

@prolic
Copy link
Member

prolic commented Aug 21, 2015

Oh now I see, I put the EventStore factory into the Stream subnamespace, my bad. I'll provide a fix ;)

prolic added a commit to prolic/event-store that referenced this issue Aug 21, 2015
@codeliner
Copy link
Member Author

@prolic yeah, I know that Zend puts factories under the Service namespace, but why? A factory is not a service. It provides a service. zend-expressive organizes the factories under a Container folder:
https://github.com/zendframework/zend-expressive/tree/master/src/Container

Maybe also a good idea. The problem I have with Service is that I expect real services in this namespace, not only factories.
And for prooph/service-bus I chose Factory: https://github.com/prooph/service-bus/tree/master/src/Factory

We must use the same namespace for factories in every project. However, if we decide fast I can change service-bus and release a bugfix

@prolic
Copy link
Member

prolic commented Aug 21, 2015

Well, Container then? :-) You decide.

@codeliner
Copy link
Member Author

Why not Factory?

@prolic
Copy link
Member

prolic commented Aug 21, 2015

As I said, you decide, it's probably the most unimportant issue to me ;-)

@codeliner
Copy link
Member Author

naming is important, but not always easy. Ok let's use Container. Zend uses it so people will look for container factories in a container folder.
I'll update service-bus.

prolic added a commit to prolic/event-store that referenced this issue Aug 21, 2015
@prolic
Copy link
Member

prolic commented Aug 21, 2015

done, see: #73

@codeliner
Copy link
Member Author

done too, see: prooph/service-bus#62

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

No branches or pull requests

2 participants