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

partial revert nullable type for curl_multi_getcontent #8367

Merged
merged 1 commit into from Sep 8, 2022

Conversation

kkmuffme
Copy link
Contributor

@kkmuffme kkmuffme commented Aug 3, 2022

Fix #8351

Partially reverts f28ac73

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Aug 3, 2022

How can the tests be fixed here? Since the reflection disagrees with the map?

@orklah
Copy link
Collaborator

orklah commented Aug 4, 2022

Can you put count back in ignoredFunctions with a comment explaining why? Please also add a test so we can make sure this change won't be reverted without a fix in the future

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Aug 4, 2022
@kkmuffme
Copy link
Contributor Author

@orklah I think you are talking about a different issue? (perhaps #8346?)

No "count" here :-) ready to merge (no extra test for this, as this is a specialty case)

@AndrolGenhald
Copy link
Collaborator

It looks like curl_multi_getcontent probably needs to go in InternalCallMapHandlerTest::$ignoredFunctions to fix the failing test.

@kkmuffme kkmuffme force-pushed the revert-nullable-curl_multi_getcontent branch from fc1abc1 to 6e4c182 Compare September 8, 2022 19:46
@kkmuffme
Copy link
Contributor Author

kkmuffme commented Sep 8, 2022

Added it to InternalCallMapHandlerTest::$ignoredReturnTypeOnlyFunctions now.
No test added, since it's in the ignored list anyway, so a test wouldn't be super useful (and a waste of my time, as this is a super niche issue)

Ready to be merged

@orklah orklah merged commit eba63a3 into vimeo:4.x Sep 8, 2022
@orklah
Copy link
Collaborator

orklah commented Sep 8, 2022

Thanks!

@kkmuffme kkmuffme deleted the revert-nullable-curl_multi_getcontent branch September 21, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nullable return types in CallMap functions technically correct but make no sense
3 participants