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
Added dependency on persistence v2. Removed docs. #9
Conversation
}, | ||
"require-dev": { | ||
"malukenho/docheader": "^0.1.7", | ||
"phpunit/phpunit": "^8.5.2", | ||
"phpspec/prophecy": "^1.10", | ||
"friendsofphp/php-cs-fixer": "^2.9.3", | ||
"doctrine/common": "^2.4", | ||
"doctrine/doctrine-orm-module": "^3.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need doctrine/doctrine-orm-module
dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To execute the test cases.
I am not very knowledgeable about Doctrine. If this is the wrong dependency, what should it be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"doctrine/persistence": "^2.0"
you added in require
section is enough I think. Also I am not sure if we should add it to required as this is optional and required only if you keep your roles using doctrine. I even thought removing it from this module it is used only in this provider https://github.com/LM-Commons/LmcRbac/blob/master/src/Role/ObjectRepositoryRoleProvider.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the dependency in require-dev
to support the test cases for the ObjectRepositoryRoleProvider
.
There was a conflict when specifying persistence v2
and "doctrine/common": "^2.4"
so I changed it to require what was needed to support the test cases.
I agree that this should be moved outside of LmcRbac
into a seperate library. The same issue exists with LmcRbacMvc
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand why you added. I'm quite familiar with the doctrine. doctrine/persistence
that was part of doctrine/common before. just saying that doctrine/persistence
should go to require-dev and doctrine/doctrine-orm-module
is not required at all I don't know why you added it.
No description provided.