search

mitchellvanw / laravel-doctrine

NO LONGER MAINTAINED! A Doctrine 2 implementation that melts with Laravel
MIT License
187 stars 74 forks source link

Remove class constant for PHP 5.4 compatibility #116

Open jasonlfunk opened 9 years ago

jasonlfunk commented 9 years ago

What is the rationale for not supporting 5.4?

Edit: Doctrine is PHP 5.4+. https://github.com/doctrine/doctrine2

jasonlfunk commented 9 years ago

And even if you don't accept it for 5.4 compatibility, accept it for consistency reasons; nothing else in the package uses "::class".

kirkbushell commented 9 years ago

Firstly - Laravel 5.1 will be 5.5+.

Secondly, we use it throughout our tests and the package on the whole (check the service provider) so saying we don't use it elsewhere, is incorrect. If there are locations where it's not being used and it makes sense to do so, then in fact THOSE areas are being inconsistent.

GrahamCampbell commented 9 years ago

Firstly - Laravel 5.1 will be 5.5+.

This isn't 100% confirmed btw. I want it to be 5.5+, but Taylor doesn't seem to want to move until the 5.2 release.

GrahamCampbell commented 9 years ago

Also, I don't see the point in supporting php 5.4 if you want to use php 5.5 features, like we are here, even if they're only minor things.

jasonlfunk commented 9 years ago

@kirkbushell It is used solely in the service provider in the master and 0.6. I was looking only in the develop branch where it isn't used at all (after my change). If you are using it in the tests, I don't see it.

But regardless, the first reason would be sufficient enough - if true. If it's not true, I would suggest not abandoning it without a good reason. Especially since both doctrine and laravel currently support it.

@GrahamCampbell What other features would be lost if you had to support 5.4?

kirkbushell commented 9 years ago

It's mostly just syntactical sugar, but it does help when things change/get refactored. I'll chat with mitch tomorrow - though we've already had this chat before and wanted to move to 5.5, regardless of other library's official support.

GrahamCampbell commented 9 years ago

I agree, we shouldn't support php 5.4.

kirkbushell commented 9 years ago

@jasonlfunk I'll chat with Mitch - there's plenty of reason to support PHP 5.4. We'll have another chat about it - you're not the only the only one to have raised it :)