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

Update PHP extension requirements & deprecate Common::mb_* methods #16754

Merged
merged 8 commits into from May 27, 2021

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 19, 2020

Description:

Our System check currently marks some PHP extensions as required that we actually don't need to require.

  • SPL
  • Reflection

Those two are meanwhile built in PHP and can't be unavailable, so doesn't make sense to check them at all.

  • mbstring

We indirectly already had symphony/polyfill-mbstring included in the composer requirements, so the methods are always available. Checking for the extension is therefor not needed anymore, as everything should work without it. (We could also consider adding this as recommended module)

  • iconv

I've added symphony/polyfill-iconv to ensure those methods are always available. We are actually not using that often, but the mbstring polyfill uses it and it allows us to remove the requirement. (We could also consider adding this as recommended module)

In addition I have marked or Common::mb_* methods as deprecated and replaced their usage with the mb_* functions directly, as those methods are actually doing nothing else. (We should do that for our plugins later as well)

refs #16650

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@sgiehl sgiehl added the Needs Review PRs that need a code review label Nov 19, 2020
@tsteur tsteur added this to the 4.2.0 milestone Nov 19, 2020
@tsteur
Copy link
Member

tsteur commented Nov 19, 2020

btw just in case it was planned be good to not replace any further upgrade methods since we don't have any issues there currently and it doesn't really improve anything.

@diosmosis diosmosis merged commit c973567 into 4.x-dev May 27, 2021
@diosmosis diosmosis deleted the requirements branch May 27, 2021 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants