Skip to content

Remove deprecated mbstring INI settings #5334

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants