-
-
Notifications
You must be signed in to change notification settings - Fork 448
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
Add autowiring for Doctrine\DBAL\Connection #685
Conversation
Although this is a class rather than an interface, it is the main API of Doctrine DBAL, which has more helper methods than the driver connection interface. So using it in typehints make sense when using these helpers.
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.
LGTM, although a test would be beneficial...
Well, we don't have tests asserting classes available for autowiring currently AFAIK. |
Yeah, but since the autowiring is squishy by nature, a regression prevention mechanism would be quite useful... |
@Ocramius I added a test ensuring the presence of our autowiring aliases, to ensure they don't get removed by mistake. I haven't tried writing a test actually performing autowiring, as this is much more complex to implement (and btw, in 3.3, the autowiring would work without this PR when you have only 1 connection defined, but would trigger a deprecation warning because making it work without the alias would involve getting through the BC layer for the old autowiring rules) |
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.
Looking good - just nitpicks missing :-)
$extension->load(array($config), $container); | ||
|
||
$expectedAliases = array( | ||
'Doctrine\DBAL\Driver\Connection' => 'database_connection', |
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.
These can all use ::class
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.
yeah, I forgot that this bundles allows it 😄
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.
done
bbad121
to
ac74c82
Compare
@mikeSimonson Dont worry i was on it, was just holding it back to see if 1.7 had issues before i merged this. Think i will do a new 1.7.x later with these changes, before 1.8 which will drop support for old symfony versions. |
@kimhemsoe Sorry, I didn't wanted to let you all the work. |
* Add autowiring for Doctrine\DBAL\Connection Although this is a class rather than an interface, it is the main API of Doctrine DBAL, which has more helper methods than the driver connection interface. So using it in typehints make sense when using these helpers. * Add a test ensuring autowiring aliases are there.
* 1.6.x: Fix deprecation with Symfony 3.4 fixed tests fixed compat with PHP 5.5 removed unused use statement changed services to be public for compat with Symfony 4 Fix check for DoctrineType::reset() existence Fixed for SF > 3.0 on shutdown container create and close connections Add "kernel.reset" tag to "form.type.entity" Bugfixes to CreateDatabaseCommand and its tests, fixes #686 fixes doctrine/orm#6689 Removing extra space Fix a possible division by zero (#634) Add autowiring for Doctrine\DBAL\Connection (#685) Use ChildDefinition class when available
Although this is a class rather than an interface, it is the main API of Doctrine DBAL, which has more helper methods than the driver connection interface. So using it in typehints make sense when using these helpers.