Remove Laravel 4 support #123
Conversation
| @@ -20,7 +17,7 @@ protected function getPackageProviders($app) | |||
| protected function getPackageAliases($app) | |||
| { | |||
| return [ | |||
| 'Sentry' => 'Sentry\SentryLaravel\SentryFacade' | |||
| 'Sentry' => 'Sentry\SentryLaravel\SentryFacade', | |||
There was a problem hiding this comment.
No since we have to support PHP 5.4 :( (for Laravel 5.0)
composer.json
Outdated
| @@ -13,11 +13,11 @@ | |||
| ], | |||
| "require": { | |||
| "php": ">=5.3.0", | |||
There was a problem hiding this comment.
Since this modification seems to break BC, maybe you should bump to "php": "^5.6||^7.0"?
It's not ideal to have an open upper constraint...
There was a problem hiding this comment.
You are correct, good one!
However I have to make this ^5.4||^7.0 since Laravel 5 works on PHP 5.4.
.travis.yml
Outdated
| @@ -20,11 +20,16 @@ env: | |||
| - LARAVEL=5.3.* TESTBENCH=3.3.* PHPUNIT=5.7.* | |||
| - LARAVEL=5.4.* TESTBENCH=3.4.* PHPUNIT=5.7.* | |||
| - LARAVEL=5.5.* TESTBENCH=3.5.* PHPUNIT=~6.0 | |||
| - LARAVEL=5.6.* TESTBENCH=3.6.* PHPUNIT=~7.0 | |||
There was a problem hiding this comment.
Are you forced to specify the PHPUnit version too? It should be automatically chosen by Composer...
There was a problem hiding this comment.
Yes, since composer.json specifies ^4.6.6 and the other 2 dependencies also so they need to be overwritten in the composer file which is what these do.
|
Seems fine to me (dropping support), though I would ask we document "On Laravel 4? Use version XXX". |
README.md
Outdated
| @@ -14,6 +14,10 @@ | |||
|
|
|||
| Laravel integration for [Sentry](https://sentry.io/). | |||
|
|
|||
| ## Laravel Version Compatibility | |||
|
|
|||
| - Laravel `4.2.x` is supported untill version `0.8.x` (`composer require "sentry/sentry-laravel:0.8.*"`) | |||
composer.json
Outdated
| @@ -13,14 +13,14 @@ | |||
| ], | |||
| "require": { | |||
| "php": "^5.4||^7.0", | |||
| "illuminate/support": "^5.0", | |||
| "sentry/sentry": "^1.8" | |||
| "illuminate/support": "5.0.*|5.1.*|5.2.*|5.3.*|5.4.*|5.5.*|5.6.*", | |||
There was a problem hiding this comment.
Maybe ^5.0, <5.7 is more readable?
|
Fix issues with the slow failing builds thanks to @agentsib (getsentry/sentry-symfony#112) |
| } | ||
| } catch (Exception $e) { | ||
| error_log(sprintf('sentry.breadcrumbs error=%s', $e->getMessage())); |
There was a problem hiding this comment.
Any chance this should be named sentry.user_context maybe, instead of …breadcrumbs?
Since Laravel 4 was released in 2013 and we have a good base for support (0.8.*) we can safely drop it IMO. So this PR does this and starts testing on Laravel 5.6.
phpcs fixer was also updated to v2 because of conflicts, I am not sure on the config but we can revisit this later and sync with
sentry-php.