[Notifier] [BC BREAK] Final classes #39557
Conversation
OskarStark
commented
Dec 18, 2020
OskarStark
commented
| Q | A |
|---|---|
| Branch? | 5.x |
| Bug fix? | no |
| New feature? | no, BC BREAK, but experimental |
| Deprecations? | no |
| Tickets | --- |
| License | MIT |
| Doc PR | --- |
There was a problem hiding this comment.
I'm ":+0:":
My general opinion about final is that's an authoritarian keyword that removes power from end users without necessarily easing maintenance - thus with no practical benefit - actually with a real drawback.
Final proved to be useful when we need to add more type hints, but here, all methods are fully typed.
|
I agree, but who knows whats coming, and the other bridges have a final keyword + the brdiges are experimental now. Just my 2cents, feel free to close |
|
BTW it helps to bring back people here if this class does not fit some of their requirements, instead of just extending the class... |
| 5.3.0 | ||
| ----- | ||
|
|
||
| * [BC BREAK] `LinkedInTransportFactory` is now final |
There was a problem hiding this comment.
Shouldn't it be reported in UPGRADE-5.3.md?
| ----- | ||
|
|
||
| * [BC BREAK] `ZulipTransport` is now final | ||
| * [BC BREAK] `ZulipTransportFactory` is now final |
OskarStark
force-pushed
the
classes-final
branch
from
073c490
to
5bd3643
Compare
|
Rebased @jderusse not sure why the lower build fails 🧐 Shall I or not add/move it to the upgrade file? I didn't do that either in the Return Type fix PR, Sorry I don't have a PR number yet, I am currently on a phone 📱 |
OskarStark
force-pushed
the
classes-final
branch
from
5bd3643
to
733ba61
Compare
|
Thank you @OskarStark. |
This PR was merged into the 5.2 branch. Discussion ---------- [Notifier] Make sure Smsapi 5.2 has a changelog | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Im not sure what the best way to fix this issue is. The smsapi bridge was released in 5.2 without a changelog. In 5.3 we added a changelog and incorrectly stated that the package was first released in 5.3. Related PR: #39557 Packagist: https://packagist.org/packages/symfony/smsapi-notifier This PR adds the changelog for 5.2 and I hope the maintainer that do the merge up handles the conflict correctly. Commits ------- 984e890 Make sure Smsapi 5.2 has a changelog
