Skip to content

Conversation

kamil-tekiela
Copy link
Member

This mostly removes copy-pasta and fixes unnoticed bugs.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

LGTM with one nit

@@ -78,8 +78,7 @@ mysqli
if (1 !== ($tmp = mysqli_affected_rows($link)))
printf("[025] Expecting int/1, got %s/%s\n", gettype($tmp), $tmp);

$charsets = array('utf8');
foreach ($charsets as $k => $charset) {
foreach (['utf8'] as $charset) {
Copy link
Member

Choose a reason for hiding this comment

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

Can't this foreach be eliminated now? Or do you want to do this as a follow-up?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to touch it further. If you'd like to find a way to remove you can do it as a follow-up. The continue statement deterred me from touching this piece of code.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, could use the typical C-ism of having a do-while loop with break statements, but lets leave this for a follow-up.

@kamil-tekiela kamil-tekiela merged commit 7e5171d into php:master Aug 17, 2024
10 checks passed
@kamil-tekiela kamil-tekiela deleted the mysqli-test-cleanup branch August 17, 2024 22:12
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