Skip to content

Zend: Deprecate non-canonical cast names #19372

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 8, 2025
Merged

Zend: Deprecate non-canonical cast names #19372

merged 2 commits into from
Aug 8, 2025

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 5, 2025

@Girgias Girgias force-pushed the deprecate-non-standard-cast-names branch from bd4a82e to b7633c0 Compare August 5, 2025 13:48
@Girgias Girgias marked this pull request as ready for review August 5, 2025 14:17
nielsdos
nielsdos reviewed Aug 5, 2025
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

You have some mixes between tabs/spaces in the parser .y file, other than that looks good

@Girgias Girgias force-pushed the deprecate-non-standard-cast-names branch from b7633c0 to 59b9ff2 Compare August 6, 2025 12:24
@Girgias
Copy link
Member Author

Girgias commented Aug 6, 2025

You have some mixes between tabs/spaces in the parser .y file, other than that looks good

I guess you meant the Zend/zend_language_scanner.l file, and thanks for spotting this seems CLion just made the default space tabs, but I'm seeing there are some others also in this file that maybe need to be fixed in a separate code style commit.

nielsdos
nielsdos approved these changes Aug 6, 2025
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Code-wise this LGTM
Question: shouldn't it say non-canonical instead of non canonical ? i.e. with a hyphen?

@Girgias
Copy link
Member Author

Girgias commented Aug 6, 2025

Code-wise this LGTM Question: shouldn't it say non-canonical instead of non canonical ? i.e. with a hyphen?

It should indeed, nicely spotted again. :) Will fix :)

@Girgias Girgias merged commit 3bf21a0 into php:master Aug 8, 2025
8 of 9 checks passed
@Girgias Girgias deleted the deprecate-non-standard-cast-names branch August 8, 2025 20:22
@jrfnl jrfnl mentioned this pull request Aug 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsdos nielsdos nielsdos approved these changes

@bukka bukka Awaiting requested review from bukka

@kamil-tekiela kamil-tekiela Awaiting requested review from kamil-tekiela

@SakiTakamachi SakiTakamachi Awaiting requested review from SakiTakamachi

Assignees
No one assigned
Labels
Category: Engine Extension: pdo_mysql Extension: phar Extension: simplexml Extension: spl Extension: standard Extension: tidy RFC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Girgias @nielsdos @DanielEScherzer