Skip to content
This repository has been archived by the owner on May 14, 2018. It is now read-only.

Remove deprecated doctrine #90

Closed
wants to merge 19 commits into from

Conversation

jhuet
Copy link
Contributor

@jhuet jhuet commented Feb 23, 2013

In order to fix #89.

It's an early PR open to suggestions. As of now it should work on its own, here's what i did :

  1. Removed references to all deprecated doctrine classes
  2. Renamed the used doctrine classes to remove Entity (as it's also usable for documents)
  3. Added an option to specify what doctrine object manager to use (again for orm / odm distinction)
  4. Updated the doctrine specific documentation

I didn't change tests though.

@Ocramius
Copy link
Contributor

@jhuet this PR is not based on current master... Did you update your fork before starting it?

if (! isset($config['bjyauthorize']['role_providers']['BjyAuthorize\Provider\Role\Doctrine']['role_entity_class'])) {
throw new InvalidArgumentException('role_entity_class not set in the bjyauthorize role_providers config.');
}
$roleClass = $config['bjyauthorize']['role_providers']['BjyAuthorize\Provider\Role\Doctrine']['role_entity_class'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep empty newlines between code blocks with different meaning and grouped assignment blocks

@Ocramius
Copy link
Contributor

Looks good so far! Git screwed up a couple of diffs, but that's not your fault :) You need a rebase anyway. Also, squash it into a single commit.

TODOs:

  • an UPGRADE.md is needed.
  • the main README.md should link doctrine's specific one

@dorongutman
Copy link

Sorry, I didn't manage to understand - it this PR should bring the mongodb integration into this module, or make sure there isn't any doctrine orm specific code in it so you could use mongodb without any custom code ?

@Ocramius
Copy link
Contributor

@dorongutman this PR removes ORM-specific hinting and old code that was deprecated. The doctrine integration was already compatible with MongoDB ODM.

@dorongutman
Copy link

@Ocramius so even though the current status' docs (before this PR) explains that I need to use the AuthenticationDoctrineEntity identity provider, I can still use it with the ODM ?

@Ocramius
Copy link
Contributor

@dorongutman correct

@Ocramius
Copy link
Contributor

@jhuet I'm overtaking this one as soon as I get home: we'll get back to 0 issues hopefully =)

@jhuet
Copy link
Contributor Author

jhuet commented Feb 28, 2013

Hmmm i just wanted to do a rebase so you could take over, but i kinda fucked up here :D I guess it's till working though.

@Ocramius
Copy link
Contributor

@jhuet np, I'll try to recover :)

@Ocramius
Copy link
Contributor

Ocramius commented Mar 1, 2013

I started working on master...cleanup;doctrine

Will close as soon as I have a new PR ready

@Ocramius
Copy link
Contributor

Ocramius commented Mar 1, 2013

Closing, see #96

@Ocramius Ocramius closed this Mar 1, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated Doctrine role and identity providers
4 participants