Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1156 +/- ##
==========================================
+ Coverage 78.36% 79.93% +1.57%
==========================================
Files 193 189 -4
Lines 28278 27715 -563
==========================================
- Hits 22159 22155 -4
+ Misses 6119 5560 -559
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| nrl_debug(NRL_FRAMEWORK, "Laravel version is " NRP_FMT, NRP_PHP(version)); | ||
|
|
||
| if (php_version_compare(version, "5.0") < 0) { | ||
| NRPRG(framework_version) = 4; |
There was a problem hiding this comment.
A quick scan of the code suggests that if we're removing framework_version && the NR_PHP_WRAPPER_REQUIRE_FRAMEWORK_VERSION macro from Laravel, we may want to remove them from the agent altogether. It doesn't appear to be used anywhere outside of the fw_laravel logic.
| {"Slim", "slim", NR_PSTR("slim/slim/app.php"), 0, nr_slim_enable, | ||
| NR_FW_SLIM}, /* 3.x */ | ||
| {"Slim", "slim", NR_PSTR("slim/slim/slim.php"), 0, nr_slim_enable, | ||
| NR_FW_SLIM}, /* 2.x */ |
There was a problem hiding this comment.
The Slim 2.x entry should be removed and the 3.x comment should be updated to 4.x
| {"Laravel", "laravel", NR_PSTR("illuminate/foundation/application.php"), 0, | ||
| nr_laravel_enable, NR_FW_LARAVEL}, | ||
| {"Laravel", "laravel", NR_PSTR("bootstrap/compiled.php"), 0, nr_laravel_enable, | ||
| NR_FW_LARAVEL}, /* 4.x */ | ||
| {"Laravel", "laravel", NR_PSTR("storage/framework/compiled.php"), 0, | ||
| nr_laravel_enable, NR_FW_LARAVEL}, /* 5.0.0-14 */ | ||
| {"Laravel", "laravel", NR_PSTR("vendor/compiled.php"), 0, nr_laravel_enable, | ||
| NR_FW_LARAVEL}, /* 5.0.15-5.0.x */ | ||
| {"Laravel", "laravel", NR_PSTR("bootstrap/cache/compiled.php"), 0, nr_laravel_enable, | ||
| NR_FW_LARAVEL}, /* 5.1.0-x */ |
There was a problem hiding this comment.
EOL'd versions of Laravel should be removed from this list
|
I think there are also Laravel integration tests that can either be removed or modified to reflect these changes- for example, Laravel has mock_artisan.php and mock_artisan.php8.php for PHP > 8.0. Since we won't support any Laravel versions that run on < PHP 8.0, we should probably remove the unsupported file and consolidate all the test logic that checks PHP version and |
Removes the code for the frameworks mentioned in the support section of https://github.com/newrelic/newrelic-php-agent/releases/tag/v12.4.0.29. To be included in a release after February