Skip to content

Rename @cname to @cvalue in stubs #9043

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 1 commit into from
Jul 19, 2022
Merged

Rename @cname to @cvalue in stubs #9043

merged 1 commit into from
Jul 19, 2022

Conversation

kocsismate
Copy link
Member

@kocsismate kocsismate commented Jul 19, 2022 •
edited
Loading

@cname currently refers to the constant name in C. However, it is not always a (constant) name, but sometimes a function invocation (https://github.com/php/php-src/pull/9043/files#diff-f4397c69dcf9f26a32970afd80fcf0ecabd726e7bc7bc1e7dfaaca48ddacf195R12), so naming it as @cvalue would be more appropriate.

TimWolla
TimWolla approved these changes Jul 19, 2022
View reviewed changes
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

The reasoning behind this PR is sound. I have verified that the _arginfo.h files only added/removed 1 line (implying there is no functional change).

cmb69
cmb69 approved these changes Jul 19, 2022
View reviewed changes
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.

Makes sense. Thank you!

@kocsismate kocsismate merged commit e328c68 into php:master Jul 19, 2022
@kocsismate kocsismate deleted the cname-to-cvalue branch July 19, 2022 13:11
@cmb69 cmb69 mentioned this pull request Jul 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimWolla TimWolla TimWolla approved these changes

@cmb69 cmb69 cmb69 approved these changes

@adoy adoy Awaiting requested review from adoy

@remicollet remicollet Awaiting requested review from remicollet

Assignees
No one assigned
Labels
Category: Build System Category: Engine Extension: calendar Extension: com_dotnet Extension: date Extension: dom Extension: enchant Extension: exif Extension: ffi Extension: fileinfo Extension: filter Extension: ftp Extension: gmp Extension: hash Extension: iconv Extension: imap Extension: json Extension: ldap Extension: libxml Extension: mbstring Extension: mysqli Extension: standard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kocsismate @TimWolla @cmb69