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

Remove class constant for PHP 5.4 compatibility #116

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

jasonlfunk
Copy link

No description provided.

@jasonlfunk
Copy link
Author

What is the rationale for not supporting 5.4?

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

@jasonlfunk
Copy link
Author

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
Copy link
Collaborator

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
Copy link

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
Copy link

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
Copy link
Author

@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
Copy link
Collaborator

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
Copy link

I agree, we shouldn't support php 5.4.

@jasonlfunk jasonlfunk closed this Mar 9, 2015
@kirkbushell kirkbushell reopened this Mar 9, 2015
@kirkbushell
Copy link
Collaborator

@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 :)

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