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

Drop IEventListener template parameter #37506

Closed
wants to merge 1 commit into from

Conversation

provokateurin
Copy link
Member

Summary

Split out from #37390

Checklist

@provokateurin provokateurin added this to the Nextcloud 27 milestone Mar 31, 2023
@come-nc
Copy link
Contributor

come-nc commented Apr 3, 2023

See vimeo/psalm#7549

@provokateurin
Copy link
Member Author

I see, so the type hint will actually be useful?

@come-nc
Copy link
Contributor

come-nc commented Apr 3, 2023

I see, so the type hint will actually be useful?

Hopefully some day. I never found a way to make it work properly.

@provokateurin
Copy link
Member Author

Hm so we either keep it and the changes in this PR to make it work in the future, or we completely drop it now since it doesn't work. I'd go with the second option, what do you think @come-nc @ChristophWurst ?

@come-nc
Copy link
Contributor

come-nc commented Apr 3, 2023

Hm so we either keep it and the changes in this PR to make it work in the future, or we completely drop it now since it doesn't work. I'd go with the second option, what do you think @come-nc @ChristophWurst ?

What happens if you keep your changes except for the ones in lib/public/EventDispatcher/IEventListener.php ?

@provokateurin
Copy link
Member Author

I think that'll work and have no effect as expected.

@provokateurin provokateurin force-pushed the feature/type-eventlisteners branch from 85d542e to fb99e51 Compare April 12, 2023 07:31
@provokateurin provokateurin changed the title Add type hints for all event listeners Drop IEventListener template parameter Apr 12, 2023
@provokateurin provokateurin added the pending documentation This pull request needs an associated documentation update label Apr 12, 2023
@provokateurin
Copy link
Member Author

@come-nc @ChristophWurst please review again.

This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv deleted the feature/type-eventlisteners branch March 14, 2024 07:50
@provokateurin provokateurin removed the pending documentation This pull request needs an associated documentation update label Jul 18, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 28 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants