Skip to content
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

Fix openspout backwards compatibility change that stops parsing formulas #156

Merged

Conversation

dakira
Copy link
Contributor

@dakira dakira commented Nov 8, 2023

Openspout 4.16 introduced a change that returns formulas verbatim instead of interpreting the result (see openspout/openspout#169).

This PR prevents this BC change for simple-excel users while introducing a new keepFormulas method that allows for the new openspout behavior.

The new minimum version of openspout has been set to 4.19 because interim versions added several fixes to the formula behavior.

Openspout 4.16 introduced a change that returns formulas verbatim instead
of interpreting the result (see openspout/openspout#169).

This PR prevents this BC change for simple-excel users while introducing
a new `keepFormulas` method that allows for the new openspout behavior.

The new minimum version of openspout has been set to 4.19 because
interim versions added several fixes to the formula behavior.
@dakira
Copy link
Contributor Author

dakira commented Nov 8, 2023

@freekmurze The CI checks are skipped because openspout requires PHP 8.1 starting with version 4.14. This fix as least requires openspout 4.16 and PHP 8.1. The PHP 8.0 tests break and make all other tests stop executing.

This will allow the tests to finish and PHP 8.0 will be completely
unsupported in a couple of days anyways.
@dakira
Copy link
Contributor Author

dakira commented Nov 8, 2023

I removed PHP 8.0 from the test pipeline to fix the checks. PHP 8.0 is end-of-life in 17 days, so I guess that's something we can live with, right?

I just saw, that Laravel 8 is EOL, too. How is that handled? Removing a Laravel version is a breaking change, right?

@freekmurze
Copy link
Member

I just saw, that Laravel 8 is EOL, too. How is that handled? Removing a Laravel version is a breaking change, right?

That isn't a breaking change, as composer will refrain from installing the package in Laravel 8 apps. No apps will break, people will just keep their current version of the package.

So, feel free drop support for older versions of Laravel if that makes these changes easier to implement.

@dakira
Copy link
Contributor Author

dakira commented Nov 9, 2023

So, feel free drop support for older versions of Laravel if that makes these changes easier to implement.

In that case, dropped it is. :-) From my point of view this is ready to merge.

@freekmurze freekmurze merged commit 4f50a47 into spatie:main Nov 9, 2023
8 checks passed
@freekmurze
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants