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

Php8 support #55

Merged
merged 7 commits into from
Jan 5, 2021
Merged

Php8 support #55

merged 7 commits into from
Jan 5, 2021

Conversation

jeger-at
Copy link
Contributor

@jeger-at jeger-at commented Nov 3, 2020

Updating to add PHP 8.0 support for #54

In order to make this repository compatible, one has to follow these steps:

  • Modify composer.json to provide support for PHP 8.0 by adding the constraint ~8.0.0
  • Modify composer.json to drop support for PHP less than 7.3
  • Modify composer.json to implement phpunit 9.3 which supports PHP 7.3+
  • Modify .travis.yml to ignore platform requirements when installing composer dependencies (simply add --ignore-platform-reqs to COMPOSER_ARGS env variable)
  • Modify .travis.yml to add PHP 8.0 to the matrix (NOTE: Do not allow failures as PHP 8.0 has a feature freeze since 2020-08-04!)
  • Modify source code in case there are incompatibilities with PHP 8.0 - Only PHPUnit 9.3 deprecation changes done

Additional changes needed

  • Update of dev dependency "dealerdirect/phpcodesniffer-composer-installer" to v0.7.0 to be composer 2 compatible (used by PHP 8 travis image)
  • moved phpunit dependency from travis.yml to composer.json
  • updated PHP versions for check-compat script in composer.json

Signed-off-by: jeger <eger.juergen@gmail.com>
Signed-off-by: jeger <eger.juergen@gmail.com>
Signed-off-by: jeger <eger.juergen@gmail.com>
Signed-off-by: jeger <eger.juergen@gmail.com>
@jeger-at
Copy link
Contributor Author

@boesing: Should I do anything else on this task? Anything I should remove again?

composer.json Outdated Show resolved Hide resolved
Signed-off-by: jeger <eger.juergen@gmail.com>
@michalbundyra
Copy link
Member

I am unsure if in this particular repository we should drop all previous PHP versions... this is migration tool for legacy ZF projects...

@froschdesign
Copy link
Member

I am unsure if in this particular repository we should drop all previous PHP versions... this is migration tool for legacy ZF projects...

I fully agree here. This tool should migrate old versions of ZF MVC applications, Apigility and Expressive.

@jeger-at
Copy link
Contributor Author

But in the end it should migrate to new laminas applications which have those old versions dropped.

@froschdesign
Copy link
Member

@jeger-at

But in the end it should migrate to new laminas applications which have those old versions dropped.

Right, but I see a different problem: the topic of upgrading the PHP version is not covered in the migration guide.
So we have to make sure that everything runs smoothly, as described in the guide.

@boesing
Copy link
Member

boesing commented Dec 8, 2020

@michalbundyra @froschdesign I share the point of @jeger-at
This package did not receive any new "feature" (as it supposed to be feature complete) since v1.2.0 which was back in late April. 7 months ago

Having a new minor which drops old PHP versions should be fine as old PHP versions are still supported by 1.2.x which still can receive bugfix backports. We could think about extending support of this package for example.

Another way would be to provide a new major version.

But I would not spend time into providing support for all versions back to PHP 5.6 as you have to use packages like the phpunit-bridge from symfony to provide support for PHPUnit versions which work on PHP 5.6 (and thus have either those type-hints on setUp and tearDown or not).

However, we are not even executing unit tests for PHP less than 7.3. Even if there would be bugs, no one will ever realize.

So imho, we should just drop old PHP versions. 🤷‍♂️

@froschdesign
Copy link
Member

@boesing

So imho, we should just drop old PHP versions.

I'm fine with it. The other versions with support for older versions are still available.

@boesing
Copy link
Member

boesing commented Dec 16, 2020

I think there is no problem with creating a new major as this package shouldnt be a dependency within other packages and thus the migration path would be quite easy (as there wont be BC breaks).

Maybe someone of the @laminas/technical-steering-committee has time to prepare this package for v2 and retarget this PR to it.

@weierophinney
Copy link
Member

@boesing I don't think we need a new major version.

The package is supposed to be used outside of a project, either via a global composer install, or by cloning. In the former case, users will install the version that is supported by the PHP version they are using. In the latter, they can choose the package version that works with their PHP version; if they choose incorrectly, Composer will be unable to install dependencies anyways. And since the tooling:

  • does not change dependency constraints
  • uses the PHP version used to invoke the tool when using composer

end users should not see any differences in usage between versions of the tooling.

As such, I'll target next minor version, as we do elsewhere.

@weierophinney weierophinney added this to the 1.3.0 milestone Jan 5, 2021
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney weierophinney merged commit f9602aa into laminas:1.3.x Jan 5, 2021
@weierophinney weierophinney linked an issue Jan 5, 2021 that may be closed by this pull request
5 tasks
@weierophinney weierophinney mentioned this pull request Jan 5, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 8.0 support
6 participants