Skip to content

Declare tentative return types in ext/spl - part 3 #7239

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

Merged
merged 2 commits into from
Jul 16, 2021

Conversation

kocsismate
Copy link
Member

No description provided.

@kocsismate kocsismate force-pushed the tentative-return-type-23 branch from b8642db to 5dea01e Compare July 14, 2021 13:33
@kocsismate kocsismate force-pushed the tentative-return-type-23 branch from 3894122 to 8eb0e76 Compare July 15, 2021 14:44
@kocsismate kocsismate merged commit b3e0888 into php:master Jul 16, 2021
@kocsismate kocsismate deleted the tentative-return-type-23 branch July 16, 2021 10:09
@jrfnl
Copy link
Contributor

jrfnl commented Jul 23, 2021

Help! It may just be me being stupid and doing something wrong, but adding the attribute to methods implementing the Iterator interfaces does not seem to work. I've done similar PRs elsewhere for other interfaces already and they all fixed the issues, but not for the Iterators.

Please do let me know if you'd rather I open a bug report at bugs.php.net.

This is where the issue can be seen:

@nikic
Copy link
Member

nikic commented Jul 23, 2021

@jrfnl I think your patch is just missing a use / FQCN.

@kamil-tekiela
Copy link
Member

@jrfnl I think it'd be best if you file a proper bug report on bugs.php.net. A comment here might get lost or forgotten.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 23, 2021

@jrfnl I think your patch is just missing a use / FQCN.

Cricky! Of course... Duh! Trying to think back now, but yes, I think the other patches I submitted for these types of things were all in non-namespaced projects ;-)

(Yeah, don't get me started, but that's another issue)

@jrfnl
Copy link
Contributor

jrfnl commented Jul 23, 2021

Confirmed: adding the use statement fixed it. Just a Friday afternoon blond moment. Thanks @nikic !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants