Skip to content

Add ext-mbstring to list of suggested dependencies - #263

Merged
schlessera merged 4 commits into
masterfrom
fix/138-composer
Jul 5, 2021
Merged

Add ext-mbstring to list of suggested dependencies#263
schlessera merged 4 commits into
masterfrom
fix/138-composer

Conversation

@swissspidy

@swissspidy swissspidy commented Jul 5, 2021

Copy link
Copy Markdown
Member

IterableCodeExtractor uses mb_ereg.

Fixes #138

@swissspidy
swissspidy requested a review from a team as a code owner July 5, 2021 13:21
@schlessera

Copy link
Copy Markdown
Member

I cannot merge this change, as it would effectively force this requirement for the entire bundled WP-CLI.

Instead of adding a hard requirement to Composer, it should be a suggested optional dependency, and the command should have a before_invoke action that checks for the presence of the extension and exits with a WP_CLI::error() if it is not present.

@swissspidy

Copy link
Copy Markdown
Member Author

Instead of adding a hard requirement to Composer, it should be a suggested optional dependency, and the command should have a before_invoke action that checks for the presence of the extension and exits with a WP_CLI::error() if it is not present.

Makes sense. Just made the necessary changes 👍

@swissspidy swissspidy changed the title Add ext-mbstring to list of required dependencies Jul 5, 2021
Comment thread composer.json Outdated
@schlessera schlessera added this to the 2.2.9 milestone Jul 5, 2021
Comment thread composer.json
@schlessera
schlessera merged commit dead2da into master Jul 5, 2021
@schlessera
schlessera deleted the fix/138-composer branch July 5, 2021 14:18
schlessera added a commit that referenced this pull request Jan 6, 2022
Add `ext-mbstring` to list of suggested dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants