-
Notifications
You must be signed in to change notification settings - Fork 13
Conversation
I like the idea. $connection = dnew('\Pdo', $this->parameters); With that we can remove constants and join constructor parameters with |
You are absolutely right! |
45540dc
to
a065588
Compare
Didn't test it. Can you read it for me please? |
I've checked out your branch into my |
I am assigning @osaris on this PR :-). |
👍 for the idea also! |
+1 for the idea, I'll check this PR when #17 is merged for a better readability. Thx. |
PR #17 has been merged. ping then? |
ping @guiled can you please rebase your PR ? Thx ! |
OK @osaris job's done! |
Good job, the concept is ok, the code looks good but I ask @Hywan for a code review mainly for Hoa guidelines (indentation, const/var names etc). Thanks. |
|
||
$connection = null; | ||
$this->parameters = [ | ||
self::DSN => $dsn, |
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.
Please fix alignment.
First, do not cache the Second, what do you think to extend this behavior to all the layers? A l'instar of Thoughts? |
ping? |
I will work on it very soon : this cache will be removed... |
/ping |
@guiled Ping? Are you available to work on this PR? If no, I guess someone can take over it. |
Any volunteer to work on this PR /cc @hoaproject/hoackers? |
ping? |
I'll look it this week. |
Perfect, thanks! |
Open the connection the later we can (not at the Dal construction, but at the first connection need).
It may makes some BC break if user test the connection result on construction.
But I think it is a good practice
NB: please consider this PR after #17 as I included the main commit about PHP5.4 migration