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

Implementation of PhpDriver, JsonDriver, YamlDriver #19

Merged
merged 16 commits into from
Sep 14, 2015

Conversation

ptheberge
Copy link
Contributor

This is a branch off of jdrich's recently accepted pull request. Wrote some code to allow for the use of configuration files for Drest rather than annotations in the Doctrine models.

@leedavis81
Copy link
Owner

This is awesome, thank you so much for your contribution. I'll have a play a little later an get this stuff merged in ASAP. Just an FYI there's a list in the v1.0.0 milestone of everything I want to achieve for a first full release (i've added this stuff to it). I'm hoping to have this completed fairly soon so you'll have something viable for production.

https://github.com/leedavis81/drest/milestones

Changes from BETA:

No longer allowed to inject the EntityManager directly. A Drest\EntityManagerRegistry must be used. A convenience method is allowed to quickly set up where only a single entity manager is needed.
Service actions can now hook onto the entity manager registry where ->getEntityManager() simple returns the default.
Removed optional support for php 5.6
Dropped support for php 5.3 (sorry, traits are really handy)
Added support for HHVM
Added support for php 7
A large number of tidy up changes
Pushed code coverage to 100%
Pushed scrutinizer quality score above 8.0
Removed injectRequest option, and always inject request object into a handle.
Add support for multiple drivers
Add YAML, PHP and JSON configuration drivers

@leedavis81
Copy link
Owner

Slightly worried about the duplication on processRoutes / processMethods in Mapping\Driver\Php and Mapping\Driver\PhpDriver Mapping\Driver\AnnotationDriver. The logic itself is identical, it just accesses config differently. i.e $route['Pattern'] instead of $route->Pattern.

Be nice if we could normalise this, possibly build a \Drest\Mapping\Annotation\Route (i know this is within the annotation namespace) object form every configuration and work the logic on that.

Not 100% sure on any drawbacks for this (memory consumption is one).

@leedavis81
Copy link
Owner

Also worth keeping an eye on: https://scrutinizer-ci.com/g/leedavis81/drest/inspections/26998928-f40c-4547-8f74-ff0bc1895f72/issues/?status=all

Trying to remove this issues entirely.

@ptheberge
Copy link
Contributor Author

I've updated the code for the drivers based off of the scrutinizer results and implemented ArrayAccess on \Drest\Mapping\Annotation\Route, moving processRoutes() to AbstractDriver.php.

processMethods() looks to be different enough to warrant PhpDriver.php having it's own implementation of the function (since there's no reliance on annotations there).

Let me know what you think!

@leedavis81 leedavis81 merged commit e305743 into leedavis81:master Sep 14, 2015
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e305743 on ptheberge:master into ** on leedavis81:master**.

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