HttplugBundle
HttplugBundle copied to clipboard
php-http
make internal classes final and mark other classes that will be final in 2.0
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | yes |
| Related tickets | prepare for #317 |
| Documentation | - |
| License | MIT |
What's in this PR?
Declaring final all classes that already are marked as @internal. Marking with @final all other classes that should be final. Making them final is technically a BC break, so we do that only in the next major.
Have all the internal classes been internal since 1.0? Otherwise making them final would technically be a BC break too.
good point. the classes have been added in #128 and have been @internal since the beginning.
What's the advantage of this? In userspace I might want to extend these classes despite that they are not covered by semver.
the problem with non-final classes is that people will start extending them, and then complain when things break because we change their signature. extending a class can break in so many ways when the base class does not consider BC (method name clashes, changing a method from protected to public in the base class, ...)
You say that, but it doesn't really happen. Not for internal classes. And when it does, it's easy to quickly close such requests. It's much more likely you will receive requests to remove those finals. Look how many of such requests are in moneyphp repository.
i do remember a few cases where people complained when we broke stuff, and i remember tons of cases where we had huge discussions about breaking BC with changes in such classes.
looking at what i want to make final, its really things about the layout in the debug toolbar. i don't expect people to need to customize that. if they want to provide visual tweaks (like you did in #355) that's awesome and should go into this repo rather than be project specific.
i do remember a few cases where people complained when we broke stuff
In classes marked @internal?
that i don't remember. but what is your goal? would you only soft-mark the classes as internal, and allow people to break the rules and when things break for them, we can tell them "told you so"?
@php-http/owners what is your take on making classes final? i prefer it, but don't have a very strong opinion...
Yes, @internal, or @final. That is defacto symfony's approach, even though recently they started to break this https://github.com/symfony/symfony/issues/25788
Yes, I don't expect need of extending these classes ever, but software engineering runs into unforeseen change of requirements all the time.