Skip to content

Remove deprecated mbstring INI settings #5334

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

Closed
wants to merge 1 commit into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Apr 1, 2020

These INI settings were deprecated back in PHP 5.6.

I tried to adapt the tests but I'm not sure if some of them aren't wrong because I'm not totally sure what the behaviour should be.

@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch 2 times, most recently from cc1efa5 to b445c11 Compare April 9, 2020 21:24
@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch from b445c11 to ed3cd54 Compare April 23, 2020 21:48
@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch 2 times, most recently from 133cbde to ac8585e Compare May 7, 2020 14:57
@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch 2 times, most recently from 1740be7 to 374dedd Compare May 9, 2020 21:03
@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch from 374dedd to 42f61fe Compare May 22, 2020 12:14
@Girgias
Copy link
Member Author

Girgias commented May 22, 2020

@nikic can you have a look at this please?

@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch from 42f61fe to b37c42b Compare July 3, 2020 10:11
@Girgias Girgias force-pushed the mbstring-remove-deprecated-ini branch from b37c42b to f015f9b Compare July 4, 2020 10:10
@Girgias
Copy link
Member Author

Girgias commented Jul 4, 2020

I've fixed the Windows compiler warning after the rebase onto master, I think this should be good to merge, can you double check @nikic?

@Girgias
Copy link
Member Author

Girgias commented Jul 20, 2020

If there are no objections I'll merge this at the end of the week.

@alexdowad
Copy link
Contributor

Hi @Girgias! Thanks very much!

I need to investigate this change a bit more to make sure I understand what all the implications will be.

(But otherwise, removing long-deprecated code is great!)

@Girgias
Copy link
Member Author

Girgias commented Oct 10, 2023

Hi @Girgias! Thanks very much!

I need to investigate this change a bit more to make sure I understand what all the implications will be.

(But otherwise, removing long-deprecated code is great!)

IIRC the reason this was not merged in 8.0 was because prior to 7.4 those settings were still actually required.

@alexdowad
Copy link
Contributor

Dear @Girgias, I really appreciate your working on this. I am just investigating this issue and believe that this patch might be more invasive than necessary or advisable.

I am working on an alternative patch and hope to show it to you soon.

@Girgias
Copy link
Member Author

Girgias commented Oct 15, 2023

Superseded by #12445

@Girgias Girgias closed this Oct 15, 2023
@Girgias Girgias deleted the mbstring-remove-deprecated-ini branch October 15, 2023 20:25
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