-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Made compatible with PHP 8.4 #4248
Conversation
migration fix for sqlite
solved the final phpunit warning and deprecation. Now ready for PHPUnit 12
Well, heck, I didn't test if it works back to the last decade. Also, I suspect that your CI is expecting only MySQL and not the oh-so-speedy SQLite, that everything is failing. |
Are you sure? I've ran the entire test suite on PHP 8.4: https://github.com/SpartnerNL/Laravel-Excel/actions/runs/11986377646/job/33419367162 and I don't get any failures or warnings in CI. If there's any deprecation warnings, we'll need to have a look at how to apply it backwards compatible. I'm open to switch to SQlite, but I'd prefer to do it in a separate PR so we just focus on getting 8.4 compatible first. Then do any improvements that are needed for sqlite, phpunit12 etc. |
When I try to install the package via composer with PHP 8.4.1 I get the following list of deprecation warnings:
|
I merged #4251 as that seems the non-breaking way of supporting it. |
Please take note of our contributing guidelines: https://docs.laravel-excel.com/3.1/getting-started/contributing.html
Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
1️⃣ Why should it be added? What are the benefits of this change?
The benefit of this change is that PHP 8.4 is now released, and this package is dead in the water on 8.4 without a fix.
2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
Well, I did add the ability to run the tests in SQLite in memory. But it's the first commit, so if this change is offensive, please delete it.
3️⃣ Does it include tests, if possible?
This is not a new feature, so no new tests were added. But the tests are all green. I even removed all of the PHPUnit warnings and deprecations. All set for PHPUnit 12.
4️⃣ Any drawbacks? Possible breaking changes?
Nope, this was artisinally delivered.
5️⃣ Mark the following tasks as done:
6️⃣ Thanks for contributing! 🙌
Pleasure doing business with you.