Laravel 13 - #60
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the library to support Laravel 12 and 13, refactors the test suite to use orchestra/testbench, and updates the documentation. Feedback was provided regarding an inconsistency between the README and composer.json version constraints for Laravel 11, as well as a recommendation to use a bounded version range for PHPUnit to prevent potential breaking changes from future major releases.
| "illuminate/log": "^11.0 | ^12.0 | ^13.0", | ||
| "illuminate/support": "^11.0 | ^12.0 | ^13.0", | ||
| "illuminate/config": "^11.0 | ^12.0 | ^13.0", | ||
| "illuminate/contracts": "^11.0 | ^12.0 | ^13.0", | ||
| "illuminate/container": "^11.0 | ^12.0 | ^13.0", | ||
| "illuminate/events": "^11.0 | ^12.0 | ^13.0", |
There was a problem hiding this comment.
There is an inconsistency between the composer.json requirements and the README.md versioning table. The README.md indicates that version ^8 of this library is intended for Laravel >= v12, while Laravel v11 should stay on version ^7. However, composer.json still includes ^11.0 in the requirements for illuminate packages. To maintain consistency and prevent Laravel 11 users from accidentally upgrading to version 8 (which may contain environment requirements or testing changes intended for newer Laravel versions), consider removing the ^11.0 constraint.
| "illuminate/log": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/support": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/config": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/contracts": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/container": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/events": "^11.0 | ^12.0 | ^13.0", | |
| "illuminate/log": "^12.0 | ^13.0", | |
| "illuminate/support": "^12.0 | ^13.0", | |
| "illuminate/config": "^12.0 | ^13.0", | |
| "illuminate/contracts": "^12.0 | ^13.0", | |
| "illuminate/container": "^12.0 | ^13.0", | |
| "illuminate/events": "^12.0 | ^13.0", |
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^10.0", | ||
| "phpunit/phpunit": ">=10.0", |
There was a problem hiding this comment.
Using an unbounded upper constraint like >=10.0 for phpunit/phpunit is generally discouraged for libraries. It may allow future major versions of PHPUnit with breaking changes to be installed, which could break your test suite or CI unexpectedly. It is safer to use a defined range that includes the versions you have verified (e.g., ^10.0 || ^11.0).
| "phpunit/phpunit": ">=10.0", | |
| "phpunit/phpunit": "^10.0 || ^11.0", |
|
Due to fluent/fluent-logger-php#71 this library doesn't work in PHP 8.4. The package is currently unmaintained. |
|
I've updated to use a fork of |
1 participant
Flags the library as usable in Laravel 13. No change to Laravel logging was made, and the library works as is.
Additional changes:
I switched from manually bootstrapping a Laravel container to using testbench. There was a change to ContextServiceProvider that broke the test because it now expects other methods to exist in the manually created container. Changes within the framework will continue to cause problems like this. testbench removes these headaches by taking care of creating a Laravel app designed for testing.