Sentry-php: We don't have PHP 5.2 support

[nokitakaze]

We already don't have PHP 5.2 support

I wanted to fix some error what I made but then I found this

So no one can do "composer tests".

https://packagist.org/packages/friendsofphp/php-cs-fixer

requires php: ^5.3.6 

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/1.13/Symfony/CS/Fixer/PSR2/EofEndingFixer.php#L43

[ste93cry]

It seems @JellyBellyDev made a fork with changes to work on PHP 5.2: immobiliare/sentry-php. What about merging the changes here?

[nokitakaze]

https://github.com/immobiliare/sentry-php/blob/master/composer.json#L19

https://packagist.org/packages/friendsofphp/php-cs-fixer#v1.8

requires: php: >=5.3.6

It's funny

[ste93cry]

It seems that the changes only involves the code of the SDK itself, not the dependencies. Plus, there is no compatible version of PHPCS with PHP 5.2. Although fork is not tested on Travis with that version of PHP, if it works it's better than nothing

[nokitakaze]

I think it is the only incompatible line with closures
https://github.com/getsentry/sentry-php/blob/master/test/Raven/Tests/ClientTest.php#L1067

But if we fix this line there still will be SDK dependecies which require >=5.3

[JellyBellyDev]

Hello guys,
php-cs-fixer and phpunit requires php >= 5.3.
Also travis does not support php5.2.
In a nutshell, is impossible to test the sdk with php52 in automatic.
In our fork we simply made the appropriate fix on sdk to work with PHP5.2 doing manual testing. :sweat:
We love sentry and we hope we have done a good thing, for those of us wants to make it work even in PHP5.2

[nokitakaze]

@JellyBellyDev, why do you still use this version?

[JellyBellyDev]

Because the legacy never dies! 😄

[Jean85]

The fact that PHP-CS is not 5.2 compatible shouldn't be an issue.
The fact that the codebase isn't, or that Travis doesn't support it, should.

Apart from that, upping the required version to 5.3 should only create issues to end users, since their Composer would only fall back to the previous, unfixed version, so we should fix this in the codebase. Also, the minimum PHP version is being bumped already in the 2.0 branch.

[Jean85]

@JellyBellyDev could you open a PR from your fork to our master to fix this issue?

[JellyBellyDev]

You say it could be accepted?
I've read some issues a while ago, did not seem interested in maintaining compatibility with 5.2.
Am I wrong?

[Jean85]

Umh, I've reviewed the differences, and I've changed my mind about this, sorry: it seems to me that the diffs are too small and mainly cause compat/conflict issues... The PR (and the consequent merges) would be more work than eventual benefits. Since we are also dropping younger PHP versions with the 2.0 development, maybe we shouldn't change anything about this.

If @nokitakaze and @ste93cry agree with me, I think we should close this as wont-fix.

[nokitakaze]

I think we should close this as wont-fix

There is no problem with Sentry does not support PHP 5.2. There is problem with Sentry does not support PHP 5.2, but declares it does.
https://github.com/getsentry/sentry-php/blob/master/composer.json#L20

[Jean85]

I completely agree. I would suggest a PR with a bump to 5.3, and an addition to the readme pointing to the fork, so troubled users would know how to solve this issue.

[JellyBellyDev]

If you want, I can do it.

[Jean85]

@JellyBellyDev I did it already, thanks.

[nokitakaze]

I do not know how to test compatibility with PHP 5.2. I could install old Ubuntu and PHP, but I don't want to write tests for such old version