Skip to content

Revert mb_trim to trim for broader PHP support#92

Merged
goetas merged 2 commits into
goetas-webservices:masterfrom
veewee:old-php-compatibility
Sep 9, 2025
Merged

Revert mb_trim to trim for broader PHP support#92
goetas merged 2 commits into
goetas-webservices:masterfrom
veewee:old-php-compatibility

Conversation

@veewee
Copy link
Copy Markdown
Contributor

@veewee veewee commented Sep 9, 2025

Reverts mb_trim changes in #91.

This was introduced by cs-fixer, but is only available in PHP 8.4+ (https://www.php.net/releases/8.4/en.php), resulting in fatal errors on lower versions.

(Sorry :-) )

@veewee veewee force-pushed the old-php-compatibility branch from 7d22281 to 2dba365 Compare September 9, 2025 10:40
@veewee veewee changed the title Rever mb_trim to trim for broader PHP support Revert mb_trim to trim for broader PHP support Sep 9, 2025
@veewee veewee force-pushed the old-php-compatibility branch from 2dba365 to a6f7c6b Compare September 9, 2025 10:40
@kernusr
Copy link
Copy Markdown

kernusr commented Sep 9, 2025

How did the tests pass with this?

@veewee
Copy link
Copy Markdown
Contributor Author

veewee commented Sep 9, 2025

I assume those code-paths are currently not covered by any tests.

@kernusr
Copy link
Copy Markdown

kernusr commented Sep 9, 2025

I checked — the code is covered by tests. However, during test execution, dev dependencies are installed, including symfony/polyfill-mbstring, which helps the tests pass.

> composer why symfony/polyfill-mbstring
friendsofphp/php-cs-fixer v3.87.1 requires symfony/polyfill-mbstring (^1.33) 
symfony/console           v7.3.3  requires symfony/polyfill-mbstring (~1.0)  
symfony/filesystem        v7.3.2  requires symfony/polyfill-mbstring (~1.8)
symfony/string            v7.3.3  requires symfony/polyfill-mbstring (~1.0)

@goetas goetas merged commit b383dc1 into goetas-webservices:master Sep 9, 2025
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.

3 participants