Skip to content

Fix array_all function description in array.c #16731

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 2 commits into from

Conversation

FraOre
Copy link
Contributor

@FraOre FraOre commented Nov 8, 2024

The prior descriptions were identical for both functions array_any and array_all, which could create confusion.

@cmb69
Copy link
Member

cmb69 commented Nov 8, 2024

Thank you for the PR! However, it seems to me that either description is not correct; it's not about finding elements, but rather if elements satisfy a given predicate. See https://wiki.php.net/rfc/array_find, https://www.php.net/manual/en/function.array-all.php and https://www.php.net/manual/en/function.array-any.php, respectively.

@FraOre
Copy link
Contributor Author

FraOre commented Nov 8, 2024

Yes, I know, I just wanted to stay as relevant as possible to the description already present.
What do you think of the new commit? Can I make squash?

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! That is fine. And no need to squash; I'll do that when applying to PHP-8.4 in a moment.

@cmb69 cmb69 closed this in 7bbf2ea Nov 8, 2024
@cmb69
Copy link
Member

cmb69 commented Nov 8, 2024

Since this is your first php-src PR: it has not been merged according to Github, but it has been applied to PHP-8.4 and master. That is how we usually handle patches which do not only go into master.

@FraOre FraOre deleted the fix-array-any-all-description branch November 8, 2024 19:37
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.

2 participants