opentracing / opentracing-php

OpenTracing API for PHP
Apache License 2.0
508 stars 56 forks source link

Migrate Interfaces to use PHP 7.1 #94

Closed jcchavezs closed 4 years ago

jcchavezs commented 4 years ago

Background

Since PHP 5.6 and 7.0 are not supported anylonger, it makes sense to update the library.

Related to https://github.com/opentracing/opentracing-php/issues/74

Ping @cawolf

piotrooo commented 4 years ago

PHP 7.1 is also not supported (https://www.php.net/supported-versions.php), we should start from 7.2.

cawolf commented 4 years ago

I will start migrating to 7.1 level. The only big difference to 7.2 will be the added object type, and the composer dependency, of course.

In the meantime, you can discuss the actual point: whether to migrate to 7.2 directly.

piotrooo commented 4 years ago

Starting with unsupported and legacy version of a PHP is not good in my opinion.

cawolf commented 4 years ago

Besides: I found only 1 file (OpenTracing\Reference), which missed a single return type hint. All interfaces are up-to-date according to both 7.1 and 7.2 specifications.

@jcchavezs , what am I missing?

jcchavezs commented 4 years ago

I think moving to 7.1 is good enough for now as it does not drag us to do retrocompatibility stuff like we did for PHP 5.6. Let's keep in mind that we should ease the migration for the implementations.

cawolf commented 4 years ago

Besides: I found only 1 file (OpenTracing\Reference), which missed a single return type hint. All interfaces are up-to-date according to both 7.1 and 7.2 specifications.

@jcchavezs , what am I missing?

ping @jcchavezs :)

jcchavezs commented 4 years ago

@cawolf I would say the best way to make sure we are not missing types is including phpstan checks. Are you up to that? For a reference you can see https://github.com/openzipkin/zipkin-php/blob/master/composer.json#L55

cawolf commented 4 years ago

Roger that :)

cawolf commented 4 years ago

related to #93