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 override method attributes #9272

Merged
merged 1 commit into from
Jun 2, 2024
Merged

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Dec 17, 2023

Helpful to detect removed/renamed parent methods of parent, possibly external, classes.

Generated based on phpstan data.

https://stitcher.io/blog/override-in-php-83 describes the advantages, for more details see https://wiki.php.net/rfc/marking_overriden_methods RFC

@alecpl
Copy link
Member

alecpl commented Dec 17, 2023

I'd like you to rather focus on fixes first, please. This is more like a feature.

@alecpl
Copy link
Member

alecpl commented Dec 17, 2023

BTW, I'm not a fan of this one.

@mvorisek mvorisek marked this pull request as draft December 18, 2023 10:03
@mvorisek mvorisek marked this pull request as ready for review January 23, 2024 00:57
@mvorisek mvorisek force-pushed the add_override_attr branch 2 times, most recently from e6a3c39 to 956e2bf Compare January 30, 2024 10:34
@mvorisek
Copy link
Contributor Author

mvorisek commented Feb 7, 2024

@alecpl, in #9272 (comment) you have written you are not fan of this change, I have updated the PR description to explain the advantages better. Did it addressed your concerns or do you still have some questions?

@mvorisek mvorisek marked this pull request as draft April 12, 2024 09:11
@mvorisek mvorisek force-pushed the add_override_attr branch from c056afc to a93a42c Compare April 12, 2024 09:19
@mvorisek mvorisek marked this pull request as ready for review April 12, 2024 09:22
@alecpl alecpl merged commit 6a5f9ee into roundcube:master Jun 2, 2024
16 checks passed
@mvorisek mvorisek deleted the add_override_attr branch June 2, 2024 14:14
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.

2 participants