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

array_change_key_case(): Throw ValueError on invalid argument #15883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Sep 14, 2024

This should be merged after 8.4 has been cut.

@Girgias Girgias marked this pull request as ready for review October 2, 2024 14:44
@Girgias Girgias requested a review from bukka as a code owner October 2, 2024 14:44
@Girgias Girgias requested a review from cmb69 October 25, 2024 21:26
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks good. Also nice to get of the contrived test cases.

@jorgsowa
Copy link
Contributor

Do we need RFC for such changes? I have similar PR in #15647

@cmb69
Copy link
Member

cmb69 commented Oct 30, 2024

I don't think such changes require an RFC (maybe in theory, but barely practical). Given #16641, we may consider to be more conservative, and deprecate passing anything but CASE_UPPER and CASE_LOWER first.

@bukka
Copy link
Member

bukka commented Nov 2, 2024

I think we should maybe put a general RFC how to deal with this. My preference is really to go through deprecations. I don't think we need to do RFC for each case but having some agreement (e.g. or possibly a single RFC) would be good.

@bukka
Copy link
Member

bukka commented Nov 2, 2024

Btw. if we agree that deprecations are better for those case, I don't think we need RFC.

@Girgias
Copy link
Member Author

Girgias commented Nov 2, 2024

Those should be warnings not deprecations if we are not turning them into ValueErrors.
I really struggle to understand the obsession of everything that is "wrong" needing to be a deprecation.

@cmb69
Copy link
Member

cmb69 commented Nov 2, 2024

I would turn them into ValueErrors with the next major version. So far users could pass arbitrary integers; PHP 8.5 deprecates passing anything else than CASE_UPPER and CASE_LOWER. In PHP 9.0 passing anything else will throw a ValueError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants