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

Drop support for PHP < 5.6? #43

Closed
rlanvin opened this issue Dec 7, 2017 · 14 comments
Closed

Drop support for PHP < 5.6? #43

rlanvin opened this issue Dec 7, 2017 · 14 comments

Comments

@rlanvin
Copy link
Owner

rlanvin commented Dec 7, 2017

For future updates I'm considering dropping support for PHP version below 5.6, since they reached end of life a long time ago.

In particular, I think this would allow me to rewrite the main method as a proper generator instead of the current awkward implementation with static variables.

Since this would break backwards compatibility, it would be a version 2.x. Version 1.x would still be available, but not supported anymore.

Anybody here still requires PHP 5.3-5.5 for some reason?

@stevebauman
Copy link

👍 I would drop PHP 5.3-5.5 support in my opinion.

Not sure why anyone would be running PHP 5.3-5.5 when PHP 7.2 has been released and PHP 5.6 has been out since August 2014.

@Sebjugate
Copy link

Sounds like a good idea to me.

rlanvin added a commit that referenced this issue Aug 7, 2018
Drop compatibility with PHP < 5.6
Ref #43
@rlanvin
Copy link
Owner Author

rlanvin commented Aug 7, 2018

I finally got the time to rewrite the code to use a generator. A quick & dirty benchmark shows a tiny performance improvement (both in speed and memory used), which is an added bonus. :-)

If some people want to help me test it in real life scenarios and/or do a code review, the code is in the generator branch. Tests on Travis are all green but since the changes are quite substantial, I'll wait a bit for some feedback before merging.

Once merged, I'll bump the major version number (2.0) since it breaks compatibility with PHP < 5.6.

@nyamsprod
Copy link

@rlanvin why not just drop PHP5 all togother and require PHP7.0 or even PHP7.1 ?

@rlanvin
Copy link
Owner Author

rlanvin commented Aug 31, 2018

@nyamsprod Because generators exists in PHP 5.6, so I don't see any technical reason to drop the support. Besides 5.6/7.0 are still supported (security patches only) until the end of the year, and I know quite a few companies that are using my lib and that are also stuck with PHP 5.6/7.0 - so why break it for them?

@nyamsprod
Copy link

@rlanvin Indeed generator exists in PHP5.6 but there are some much more to dropping PHP5 line. First it reduces maintenance cost since you don't need to worry about old/deprecated PHP versions (security fix means that only security fix are being fixed not all bugs).

You want to release a new MAJOR version so why not take advantage of PHP7 features like

  • scalar typehint;
  • constant visibility; (PHP7.1)
  • return type;
  • void returns (PHP7.1)
  • DateTimeInterface constant (PHP7.2)
  • null coalescence
  • .. and the list goes on

Also if you intent is to release a MAJOR new version you could even take the time to also refactor the code and implement some coding standard. That too would be a major incentive in making those companies upgrade to PHP7.

Don't forget that the current version still works, so those that don't want or can't upgrade to PHP7 will be fine with the current release and won't upgrade anyway (composer will make sure of that). They may not receive improved features but that their decision not yours 👍 .

As a maintainer your first duty is does this code works fine for me in what I do with it. If the answer is I would like my code to work even in PHP5.6 then fine by me being a library maintainer myself I do understand that It takes time to do all this but dropping PHP5 or even PHP7.0 should reduce such time IMHO.

@rlanvin
Copy link
Owner Author

rlanvin commented Aug 31, 2018

@nyamsprod Thanks for the lesson about being a maintainer, but I guess we have a different view on what it means. For me, backward compatibility is more important than new syntactic sugar. If it ain't broken, don't fix it. My "first duty" as a maintainer, as you put it, is to think about others (the users of the lib) not myself.

This lib is being used by a lot of different projects and companies and I feel I have a responsibility to keep it compatible for as many versions as I can, even if that means that I need to restrict myself slightly in the way I code. I don't see a value in breaking backward compatibility (and leaving many developers stuck with an unmaintained 1.x version of the lib), just so I can use a scalar typehint, or write ?? instead of isset() ? :... In my opinion, a decision to drop support for a not-yet-EOL version of PHP has to be carefully considered and has to bring more benefits than simply "hey, the code would look more modern".

That said, if you have such a benefit in mind (remember, something that would bring value for the users of the lib) please share!

@nyamsprod
Copy link

@rlanvin TBH I don't care about PHP7 syntactic sugar what I care is having a major release with value. Some BC break could be introduce to improve the code and the library usage regardless of PHP7 syntactic sugar. IMHO dropping PHP5 means less headache but if you are willing to support it then your the maintainer you decide (ie PHP7.0 and PHP5.6 will be EOL in less than 5 month which means that in 6 months you'll release another major version 🤔 ).

For Backward compatibility only supporting PHP7 does not mean BC as for going from array to generator is BC break anyway.

Something that could be added is a Exception marker for your library. As it stand one has to filter the SPL exception based on their messages If you would add a Exception Marker one could simply typehint the marker to easily catch the library exception.
These are the type of BC a major release could/should introduce.
As of PHP7 only benefit the firsts that came to mind are:

  • return type
  • scalar typehint
  • strict type
    By enforcing those without changing your current codebase you improve the code, the test suite and the maintenance.

Again it's your call but here's some area of improvement. Also pick a coding standard to help potential contributor submit better PR.

@rlanvin
Copy link
Owner Author

rlanvin commented Aug 31, 2018

@nyamsprod Alright I understand what you're saying and I think there is a misunderstanding here. If I was to break BC for version 2, then yes, sure, I'll agree with you, let's redo everything now so there is no need to break everything again in 6 months.

However I'm not going from array to generator (I don't know where you got that from). I'm going for a generator implementation that works with PHP 5.3+, to the native PHP generator implementation that only works with PHP 5.6+. It's purely an implementation change. The class interface doesn't change, so if you were already using PHP 5.6 or above, there will be no breakage at all, it'll just be a drop-in replacement. It will only break for you if you were using PHP < 5.6, which hopefully nobody is anymore.

Your suggestion about the Exception is definitely interesting, but that's exactly the kind of things I'm trying to avoid at the moment, because it will break of lot of existing code for a lot of people. Suddenly, all the try... catch will not work anymore because the exception class has changed and people will need to spend time (and company's money) to upgrade their code, it's a pain in the ass, etc. So unless it's a real problem for many people, it'll wait for when I actually decide to break BC for everybody.

@nyamsprod
Copy link

Dropping support for a PHP version does not equal to BC break so if you only change internal code that is not BC... as long as your classes which implement the interface are final go ahead release a minor version and your good to go otherwise if someone has extended your classes it's a subtle BC break. And I don't get the BC break worry it's composer role to guard against incompatible package so it should prevent any conflict between package version

@rlanvin
Copy link
Owner Author

rlanvin commented Sep 1, 2018

@nyamsprod Look this discussion is going in circles, you are answering your own questions. It'll break for people still using PHP < 5.6 (if any) and indeed for people who have extended the classes (if any). Therefore it has to be a major version release, that's not up for debate. I hope as few people as possible will be concerned though.

So thank you for all your suggestions, but as I explained my philosophy as a maintainer is to try to minimize the impact of updates and avoid any unnecessary disruption for the users of the lib. I'll continue to support PHP 5.6/7.0 at the very least until the official EOL (or until it becomes impossibly hard to do so).

@nyamsprod
Copy link

@rlanvin thanks for the clarification while I don't necessarily agree you are the one making the final call so I'll respect your decision.

@boenrobot
Copy link

boenrobot commented Sep 12, 2018

I'd like to add myself as a person, who's projects are stuck with PHP 5.6... I'm considering using this library for a feature we're developing at the company I work for, but our platform was written long ago, by people who no longer work at the company, using Zend Framework 1, and is too big and messy to simply be rewritten in anything PHP 7 compatible.

Not to mention that the company's management always wants new features (and wants them developed fast), and invests in this over performance, security and stability, so we can't even be like "right... let's keep the production code running in ZF1 while we migrate to something else".

So if PHP 5 support was dropped completely, we'd be stuck with whatever the last compatible version is.

rlanvin added a commit that referenced this issue Jan 13, 2019
Drop compatibility with PHP < 5.6
Ref #43
@rlanvin
Copy link
Owner Author

rlanvin commented Jan 13, 2019

I just published version 2.0.0-rc1. Happy to get some feedback.

@rlanvin rlanvin closed this as completed Mar 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants