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

Add return type hint to ViewModel::getIterator() #220

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Add return type hint to ViewModel::getIterator() #220

merged 1 commit into from
Oct 17, 2023

Conversation

func0der
Copy link
Contributor

This is to satisfy static code analytic tools like PHPStan and Psalm.

Resolves #73 and also helps with #56

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks @funcoder 👍

Really, this should be Traversable<int, ModelInterface> to fit with the return type of IteratorAggregate - when we eventually get return types added in 3.0, This will receive Traversable as its native return type.

Also, can you please update and target the latest minor 2.31.x. Your change is not a BC break so it can be released in a minor.

@gsteel
Copy link
Member

gsteel commented Oct 17, 2023

I should also mention that to keep psalm happy here, you'll need to annotate $this->children as list<ModelInterface>

@gsteel gsteel added this to the 2.31.0 milestone Oct 17, 2023
@func0der func0der changed the base branch from 3.0.x to 2.31.x October 17, 2023 20:25
@func0der
Copy link
Contributor Author

🙄 Github interface confused me with the branches xD

I hope I got everything the way you wanted it to be, @gsteel ?

@func0der func0der requested a review from gsteel October 17, 2023 20:30
This is to satisfy static code analytic tools like PHPStan and Psalm.

Resolves #73

Signed-off-by: func0der <[email protected]>
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @func0der

@gsteel gsteel merged commit 6188b66 into laminas:2.31.x Oct 17, 2023
9 checks passed
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.

Type hint for ViewModel::getIterator()
2 participants