[RFC] Domain managers #1764
[RFC] Domain managers #1764
Conversation
|
Nice, moving the events and persisting to manager sounds good. What do you think about moving the creation logic to another type of service "factory"? That would keep everything even more separated and allow easy overriding/adding new kinds of |
|
👍 |
|
@pjedrzejewski I think that could be next step on the route to separate all the thing(tm) =) This PR IMO should be not doing that much, as it would be kinda painful to migrate after merge. So I guess that "factory" is quite interesting idea, but for further improvements =) |
|
What is the status of this ? |
stloyd
force-pushed
the
feature/domain_managers
branch
3 times, most recently
from
fae588b
to
fe93b00
Compare
stloyd
force-pushed
the
feature/domain_managers
branch
6 times, most recently
from
b17ba29
to
2783521
Compare
|
@Arn0d I guess it's RFR now ;) |
stloyd
force-pushed
the
feature/domain_managers
branch
4 times, most recently
from
6a6381d
to
0f116d2
Compare
stloyd
force-pushed
the
feature/domain_managers
branch
3 times, most recently
from
0565a2a
to
f2d3780
Compare
stloyd
force-pushed
the
feature/domain_managers
branch
from
f2d3780
to
2f9753c
Compare
|
well i'm happy for the feature. I hope something like this gets accepted. |
stloyd
force-pushed
the
feature/domain_managers
branch
from
2f9753c
to
28a5f70
Compare
|
@pjedrzejewski @Arn0d @umpirsky @kayue I guess this is ready for review... |
stloyd
force-pushed
the
feature/domain_managers
branch
from
dec1dde
to
ec841f2
Compare
stloyd
force-pushed
the
feature/domain_managers
branch
from
ec841f2
to
2af985a
Compare
stloyd
force-pushed
the
feature/domain_managers
branch
from
2af985a
to
5a2abcc
Compare
|
Looking forward to this! |
Adds to Sylius/Sylius#1764 as that is first spot I don't notice need for doctrine bundle. We use custom ORM and therefore don't include doctrine
stloyd
force-pushed
the
feature/domain_managers
branch
2 times, most recently
from
85f40e3
to
788dcfd
Compare
|
so will this get merged as is? i think i'm about ready to use this as a patch to my project. |
| $manager = $this->manager; | ||
| if ($manager instanceof EntityManagerInterface) { | ||
| $manager->transactional(function ($manager) use ($resource, $action) { | ||
| $manager->{$action}($resource); |
There was a problem hiding this comment.
It would be nice to test the $action method exists before, right ?
There was a problem hiding this comment.
Look few lines above...
stloyd
force-pushed
the
feature/domain_managers
branch
2 times, most recently
from
c08800b
to
3991496
Compare
|
@pjedrzejewski Feedback? |
stloyd
force-pushed
the
feature/domain_managers
branch
2 times, most recently
from
936d5c6
to
8ec4e37
Compare
|
yes. feedback please :) |
stloyd
force-pushed
the
feature/domain_managers
branch
from
8ec4e37
to
6c7bc17
Compare
|
@stloyd @pjedrzejewski news about this topic ?? |
|
@pjedrzejewski What about this PR ? |
|
Replaced by #2255. |
As many time mentioned (not only me) principale of creating things is not job for repositories, theirs job is finding. So I finally find some time to introduce "proper" managers to do the job right.