Propel\Runtime\Adapter\Pdo\PdoConnection doesn't implement ConnectionInterface #71

Closed
nnarhinen opened this Issue Nov 22, 2011 · 27 comments

Comments

Projects
None yet
4 participants
@nnarhinen
Contributor

nnarhinen commented Nov 22, 2011

I get error

PHP Fatal error:  Class Propel\Runtime\Adapter\Pdo\PdoConnection contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Propel\Runtime\Connection\ConnectionInterface::inTransaction) in /vagrant/src/Propel/Runtime/Adapter/Pdo/PdoConnection.php on line 52

When trying to run the test suite.

@francois it seems to have something to do with your PR #64 maybe

@nnarhinen

This comment has been minimized.

Show comment Hide comment
@nnarhinen

nnarhinen Nov 22, 2011

Contributor

Ok so I got the username wrong, I meant @fzaninotto of course.. sorry.

Contributor

nnarhinen commented Nov 22, 2011

Ok so I got the username wrong, I meant @fzaninotto of course.. sorry.

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Nov 22, 2011

Member

Which PHP version do you have ? http://www.php.net/manual/fr/pdo.intransaction.php
You need at least 5.3.3 obviously

Member

willdurand commented Nov 22, 2011

Which PHP version do you have ? http://www.php.net/manual/fr/pdo.intransaction.php
You need at least 5.3.3 obviously

@nnarhinen

This comment has been minimized.

Show comment Hide comment
@nnarhinen

nnarhinen Nov 22, 2011

Contributor

Oh, this ubuntu box has only 5.3.2.. That should then maybe be stated somewhere..

Contributor

nnarhinen commented Nov 22, 2011

Oh, this ubuntu box has only 5.3.2.. That should then maybe be stated somewhere..

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Nov 22, 2011

Member

Yep, the README says "Propel2 is only supported on PHP 5.3.2 and up." :o

I'm gonna changing it. thanks!

Member

willdurand commented Nov 22, 2011

Yep, the README says "Propel2 is only supported on PHP 5.3.2 and up." :o

I'm gonna changing it. thanks!

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Nov 23, 2011

Member

er... I'm willing to remove the inTransaction() method from the interface if it can make Propel more compatible with older versions of PHP. As far as I know, inTransaction() is used nowhere in the Propel code...

Member

fzaninotto commented Nov 23, 2011

er... I'm willing to remove the inTransaction() method from the interface if it can make Propel more compatible with older versions of PHP. As far as I know, inTransaction() is used nowhere in the Propel code...

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Dec 12, 2011

Member

@fzaninotto Did you change something here ? Or we keep PHP 5.3.3 as minimum version.

Member

willdurand commented Dec 12, 2011

@fzaninotto Did you change something here ? Or we keep PHP 5.3.3 as minimum version.

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Dec 12, 2011

Member

Haven't done anything yet, let's keep this ticket open as a reminder. I'll assign it to me.

Member

fzaninotto commented Dec 12, 2011

Haven't done anything yet, let's keep this ticket open as a reminder. I'll assign it to me.

@ghost ghost assigned fzaninotto Dec 12, 2011

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Mar 7, 2012

Member

ping @fzaninotto, we should decide whether to level up the PHP version or not

Member

willdurand commented Mar 7, 2012

ping @fzaninotto, we should decide whether to level up the PHP version or not

@willdurand willdurand closed this Mar 7, 2012

@willdurand willdurand reopened this Mar 7, 2012

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Apr 1, 2012

Member

I think to set the minimum version to PHP 5.3.3 is not critical. So, could we level up the version?

Member

willdurand commented Apr 1, 2012

I think to set the minimum version to PHP 5.3.3 is not critical. So, could we level up the version?

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Apr 2, 2012

Member

up to you. You should find some stats to back up your decision on usage facts.

Member

fzaninotto commented Apr 2, 2012

up to you. You should find some stats to back up your decision on usage facts.

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Apr 14, 2012

Member

5.3.0 : 1,810
5.3.1 : 2,715
5.3.2 : 21,890
5.3.3 : 28,359
5.3.4 : 5,261
5.3.5 : 16,680
5.3.6 : 31,549
5.3.7 : 643
5.3.8 : 15,310

From: http://blog.pascal-martin.fr/post/statistiques-versions-php-2011-09, on september 2011.

Actually, I think that Propel2 should be 5.4 ready. It's another discussion but what about having AR traits for instance?
People could stop complaing on inheritance, and base classes, blablabla...

Member

willdurand commented Apr 14, 2012

5.3.0 : 1,810
5.3.1 : 2,715
5.3.2 : 21,890
5.3.3 : 28,359
5.3.4 : 5,261
5.3.5 : 16,680
5.3.6 : 31,549
5.3.7 : 643
5.3.8 : 15,310

From: http://blog.pascal-martin.fr/post/statistiques-versions-php-2011-09, on september 2011.

Actually, I think that Propel2 should be 5.4 ready. It's another discussion but what about having AR traits for instance?
People could stop complaing on inheritance, and base classes, blablabla...

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Apr 14, 2012

Member

I think PHP 5.4 adoption will take a year at least to reach a good 30%. It's too early to make the jump. Also, most people will come to Propel from Symfony2, which is PHP 5.3. Asking them to upgrade PHP in order to be able to use Propel is a no-go.

Having alternative behavior implementations using traits is a nice to have for those who support PHP 5.4, but I doubt this can improve the performance. You just need to see all the processing done in the behavior builders: all that would have to be done at runtime!

Member

fzaninotto commented Apr 14, 2012

I think PHP 5.4 adoption will take a year at least to reach a good 30%. It's too early to make the jump. Also, most people will come to Propel from Symfony2, which is PHP 5.3. Asking them to upgrade PHP in order to be able to use Propel is a no-go.

Having alternative behavior implementations using traits is a nice to have for those who support PHP 5.4, but I doubt this can improve the performance. You just need to see all the processing done in the behavior builders: all that would have to be done at runtime!

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Apr 14, 2012

Member

The idea is more to generate traits :)

Member

willdurand commented Apr 14, 2012

The idea is more to generate traits :)

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Apr 14, 2012

Member

Ah, I see. Then it's a very good idea. But not for this year IMHO.

2012/4/14 William DURAND <
reply@reply.github.com

The idea is more to generate traits :)


Reply to this email directly or view it on GitHub:
#71 (comment)

Member

fzaninotto commented Apr 14, 2012

Ah, I see. Then it's a very good idea. But not for this year IMHO.

2012/4/14 William DURAND <
reply@reply.github.com

The idea is more to generate traits :)


Reply to this email directly or view it on GitHub:
#71 (comment)

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Apr 14, 2012

Member

I have to look a bit more on traits, but a config parameter could be added to determine whether you want AR traits, or inheritance.

Anyway, PHP 5.3.3 seems ok, right?

Member

willdurand commented Apr 14, 2012

I have to look a bit more on traits, but a config parameter could be added to determine whether you want AR traits, or inheritance.

Anyway, PHP 5.3.3 seems ok, right?

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Apr 14, 2012

Member

Do you mean "leaving behind 22% of users who can run Symfony2 but not Propel2"? I personally think it's too much.

Member

fzaninotto commented Apr 14, 2012

Do you mean "leaving behind 22% of users who can run Symfony2 but not Propel2"? I personally think it's too much.

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Apr 15, 2012

Member

Well, you're right. So.. any suggestions?

2012/4/14 Francois Zaninotto <
reply@reply.github.com

Do you mean "leaving behind 22% of users who can run Symfony2 but not
Propel2"? I personally think it's too much.


Reply to this email directly or view it on GitHub:
#71 (comment)

Member

willdurand commented Apr 15, 2012

Well, you're right. So.. any suggestions?

2012/4/14 Francois Zaninotto <
reply@reply.github.com

Do you mean "leaving behind 22% of users who can run Symfony2 but not
Propel2"? I personally think it's too much.


Reply to this email directly or view it on GitHub:
#71 (comment)

fzaninotto added a commit to fzaninotto/Propel2 that referenced this issue Apr 28, 2012

Remove `inConnection()` from the `ConnectionInterface`
This brings back the compatibility ith Propel 5.3.2

Refs #71

@willdurand willdurand closed this May 1, 2012

@willdurand willdurand reopened this May 7, 2012

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand May 7, 2012

Member

Symfony2 just raised the minimal version of PHP to 5.3.3, we could do the same. What do you think?

symfony/symfony@3719c70
symfony/symfony#2985

Member

willdurand commented May 7, 2012

Symfony2 just raised the minimal version of PHP to 5.3.3, we could do the same. What do you think?

symfony/symfony@3719c70
symfony/symfony#2985

@hhamon

This comment has been minimized.

Show comment Hide comment
@hhamon

hhamon May 7, 2012

Member

+1 for me

Member

hhamon commented May 7, 2012

+1 for me

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto May 9, 2012

Member

Depends. Is there a PropelBundle for Propel2/SF2.0 in the works?

Member

fzaninotto commented May 9, 2012

Depends. Is there a PropelBundle for Propel2/SF2.0 in the works?

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand May 9, 2012

Member

No.

2012/5/9 Francois Zaninotto <
reply@reply.github.com

Depends. Is there a PropelBundle for Propel2/SF2.0 in the works?


Reply to this email directly or view it on GitHub:
#71 (comment)

Member

willdurand commented May 9, 2012

No.

2012/5/9 Francois Zaninotto <
reply@reply.github.com

Depends. Is there a PropelBundle for Propel2/SF2.0 in the works?


Reply to this email directly or view it on GitHub:
#71 (comment)

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto May 9, 2012

Member

Then it's only a matter of deciding to leave behind 22% of current PHP users.

But is there a requirement for a version bump?

Member

fzaninotto commented May 9, 2012

Then it's only a matter of deciding to leave behind 22% of current PHP users.

But is there a requirement for a version bump?

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand May 9, 2012

Member

As Symfony2 components require 5.3.3 now, I'm pretty sure it will break things if we don't change the minimal version for Propel2 itself.
That said, I don't want to change the version "just" because of Symfony2 (even if it's probably better). My point is: we have now two arguments to increase the minimal version.

Member

willdurand commented May 9, 2012

As Symfony2 components require 5.3.3 now, I'm pretty sure it will break things if we don't change the minimal version for Propel2 itself.
That said, I don't want to change the version "just" because of Symfony2 (even if it's probably better). My point is: we have now two arguments to increase the minimal version.

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Jun 15, 2012

Member

@fzaninotto could you add the method you removed, and bump the requirement to PHP 5.3.3? (doc + composer)

Member

willdurand commented Jun 15, 2012

@fzaninotto could you add the method you removed, and bump the requirement to PHP 5.3.3? (doc + composer)

@fzaninotto

This comment has been minimized.

Show comment Hide comment
@fzaninotto

fzaninotto Jun 15, 2012

Member

That basically means replaying backwards? My git knowledge is rusty on this side, and I won't have time to setup a Propel2 checkout for at least a few days.

Member

fzaninotto commented Jun 15, 2012

That basically means replaying backwards? My git knowledge is rusty on this side, and I won't have time to setup a Propel2 checkout for at least a few days.

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Jun 15, 2012

Member

No problem. I will do that then.

Le 15 juin 2012 à 18:26, Francois Zaninotto reply@reply.github.com a écrit :

That basically means replaying backwards? My git knowledge is rusty on this side, and I won't have time to setup a Propel2 checkout for at least a few days.


Reply to this email directly or view it on GitHub:
#71 (comment)

Member

willdurand commented Jun 15, 2012

No problem. I will do that then.

Le 15 juin 2012 à 18:26, Francois Zaninotto reply@reply.github.com a écrit :

That basically means replaying backwards? My git knowledge is rusty on this side, and I won't have time to setup a Propel2 checkout for at least a few days.


Reply to this email directly or view it on GitHub:
#71 (comment)

@willdurand

This comment has been minimized.

Show comment Hide comment
@willdurand

willdurand Jun 18, 2012

Member

done

Member

willdurand commented Jun 18, 2012

done

@willdurand willdurand closed this Jun 18, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment