Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

WIP: #99 - remove php 5.6, 7.0 and 7.1 support #100

Closed
wants to merge 6 commits into from
Closed

WIP: #99 - remove php 5.6, 7.0 and 7.1 support #100

wants to merge 6 commits into from

Conversation

dennybrandes
Copy link

@dennybrandes dennybrandes commented Mar 22, 2019

  • remove php 5.6 and 7.0
  • change .travis config

@dennybrandes dennybrandes changed the title WIP: #99 - remove php 5.6 and php 7.0 suppoer WIP: #99 - remove php 5.6 and php 7.0 support Mar 22, 2019
composer.json Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@dennybrandes dennybrandes changed the title WIP: #99 - remove php 5.6 and php 7.0 support WIP: #99 - remove php 5.6, 7.0 and 7.1 support Mar 22, 2019
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a BC break, and the title does not reflect what it is doing:

  • Bumping the minimum supported version of PHP to 7.2
  • Incorrectly removing all Travis environments except 7.3 (it MUST keep at least 7.2 if it will still target 7.2!)
  • Adopting PHP 7.1+ features, including strict_types, return type hints and scalar type hints

Regarding the latter, what is done is incomplete, and only addresses a small subset of the classes in this library.

The only way I'll accept this is if it does a thorough update to PHP 7.2 across all classes. Doing it piecemeal across multiple patches will inevitably leave us with classes and interfaces that do not have all type hints reflected correctly, and/or where we haven't done work to ensure type hints can be used (e.g., modifying AbstractOptions and/or ParameterObjectInterface to be iterable, so that setFromArray() can become setFromIterable(iterable $options) : AbstractOptions). Any API changes also MUST be documented in a migration document.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants