[HttpKernel][DataCollectorInterface] Ease compatibility #34230
Conversation
There was a problem hiding this comment.
(once comment is addressed)
This is allowed by the BC promise - and is the only way to make this argument accept Throwable in Symfony 5.0, while still allowing third-party bundles to be compatible with 3.4 (since most are as of now)
src/Symfony/Component/HttpKernel/DataCollector/DataCollectorInterface.php
Outdated
Show resolved
Hide resolved
7154e1c to
55f85b2
Compare
|
After talking with @fabpot, I'm going to merge this change on 4.3. |
55f85b2 to
761df46
Compare
|
Thank you @fancyweb. |
…(fancyweb) This PR was submitted for the 3.4 branch but it was merged into the 4.3 branch instead. Discussion ---------- [HttpKernel][DataCollectorInterface] Ease compatibility | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | https://github.com/symfony/symfony/pull/33065/files#r342109866 | License | MIT | Doc PR | - To ease data collectors being compatible from 4.3 to 5.0. Commits ------- 761df46 [HttpKernel][DataCollectorInterface] Ease compatibility
…-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [DoctrineBridge] fix min version of http-kernel | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Required after #34257 and #34230 Commits ------- 69ba86b [DoctrineBridge] fix min version of http-kernel
Actually, the note on the BC promise page is inaccurate. Only an optional argument could be removed: Works: https://3v4l.org/voGVW |
|
Correct, can you please send a PR to the corresponding doc page? |
…hanhui) This PR was merged into the 3.4 branch. Discussion ---------- BC promise: only optional arguments can be removed Fixes symfony/symfony#34230 (comment) I've changed `Yes` to `No`, since it makes more sense, i.e. it's not allowed, unless... Additional question: Should I also change `No` to `No [3]_` for traits? Or is there a specific reason why it's absolutely not allowed on traits? Commits ------- ef477e3 BC promise: only optional arguments can be removed
|
The fact that this change wasn't merged into 3.4 makes supporting Symfony 3.4 in a bundle unnecessarily complicated: Symfony 5 and Symfony 3.4 can't be supported in the same version due to this incompatibility. @fabpot what was the reason to only merge this into 4.3, but not 3.4? |
|
I should have merged it in 4.4 as this is not a bug fix. I agreed to merge it in 4.3 to ease the migration path. Our goal is that third-parties can support 2 consecutive versions (3 and 4 OR 4 and 5). Supporting 3, 4, and 5 is not possible. |
To ease data collectors being compatible from 4.3 to 5.0.