Skip to content

WIP: Enable different entity managers for multiple log entities#44

Open
hason wants to merge 1 commit into
xiidea:masterfrom
hason-contributions:multiple
Open

WIP: Enable different entity managers for multiple log entities#44
hason wants to merge 1 commit into
xiidea:masterfrom
hason-contributions:multiple

Conversation

@hason

@hason hason commented Nov 27, 2019

Copy link
Copy Markdown

WIP

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-48.8%) to 51.176% when pulling 3f24213 on hason-contributions:multiple into 4e035c6 on xiidea:master.

@ronisaha ronisaha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have to consider ODM also.


/**
* @var ManagerRegistry
* @var \Doctrine\ORM\EntityManager

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As we also support ODM, we don't want direct dependency with ORM. You can see we remove the dependency in a earlier commit

Comment thread composer.json
"php-coveralls/php-coveralls": "^2.1",
"doctrine/common": ">=2.2 <3.0"
"doctrine/common": ">=2.2 <3.0",
"doctrine/orm": "^2.2"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No dependency should be with ORM

Comment thread Logger/Logger.php
{
$this->getManager()->persist($event);
$this->getManager()->flush($event);
$manager = $this->doctrine->getManagerForClass($event);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

getManagerForClass expect class name not an object

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.

3 participants