-
Notifications
You must be signed in to change notification settings - Fork 282
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 PHP 5.3 support #3213
Drop PHP 5.3 support #3213
Conversation
1e347e4
to
7e20dc0
Compare
Rationale: #3186 (comment) |
I agree with you that it is time to drop 5.3. We announced this with 2.5.0, but we failed to enforce it. Changing this in a patch version might break existing setups that didn't listen and still run 2.5.0 or 2.5.1 on 5.3. It's not supported, sure. But it's not a good move to break things in a minor release. When merging this I'd first create a support/2.5 branch like we did in other projects. This would allow us still backport important fixes to a 2.5.2 in a safe and compatible way while moving on with the master. This would however influence the release policy for Icinga Web 2, and it probably also depends on the schedule for 2.6. @lippserd: it's your choice! |
7e20dc0
to
4eef886
Compare
4eef886
to
fc1f6e1
Compare
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.
LGTM 👍
@@ -568,11 +568,10 @@ public function getRequirements($skipModules = false) | |||
$set = new RequirementSet(); | |||
|
|||
$set->add(new PhpVersionRequirement(array( | |||
'condition' => array('>=', '5.3.2'), | |||
'condition' => array('>=', '5.4'), |
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.
@lippserd Aren't we using 5.6 as our new requirement?
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.
Yes, will be in a new PR. Did not want to mix things here.
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.
No description provided.