[HttpFoundation] Add support for DateTimeImmutable in setters #19727
Conversation
Backwards compatibility is assured by typehinting for DateTimeInterface, and converting to \DateTime for onward use
|
The problem is that when someone extends the response and the methods you change the signature of, it will cause compatibility issues afaik. |
|
Urgh, I suppose that is indeed true although I can't imagine anyone actually extending those methods. If I targeted this to master would you take it? |
|
The master would still mean it's a BC break and that cannot be accepted :( |
|
Same as #19578 These changes need a |
No, there never can be a BC break in Symfony. |
|
4.0 will have BC breaks..? Anyway, currently Having a branch targeting 4.0 is considerable as you can actually make these changes somewhere and you prepare the release beforehand. Pro: actually have your upcoming version in VCS |
|
Method signatures other than constructors can't really be provided via a BC layer |
|
I would consider this BC break in 4.0 :) but it can't be done. Lets move forward :) edit: we could consider documenting the break now, and not forget to make the change in 4.0 (like not forgetting to remove the deprecations, etc.) |
| * | ||
| * @return Response | ||
| */ | ||
| public function setDate(\DateTime $date) | ||
| public function setDate(\DateTimeInterface $date) |
There was a problem hiding this comment.
What about renaming it to setDateHeader for bc ?
There was a problem hiding this comment.
technically, it would work yes
There was a problem hiding this comment.
Another solution could be to deprecate extending this functions and make them final in 4.0 (i prefer this solution).
There was a problem hiding this comment.
@Ener-Getick let's give it a try, would you mind opening a PR doing this?
…r-Getick) This PR was merged into the 3.2-dev branch. Discussion ---------- [HttpFoundation] Deprecate extending some methods | Q | A | ------------- | --- | Branch? | "master" | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #19727 | License | MIT | Doc PR | It's really hard to change methods signature because of bc. I'm proposing to deprecate extending some getters/setters of `Response` because of this (and because extending them is not really useful). If you like this approach it could be used in other places to simplify bc in 4.0. Edit: This causes issues (warnings always triggered) when mocking `Response` entirely but does it matter as people should only mock needed methods? Commits ------- c0a26bc [HttpFoundation] Deprecate extending some methods
…r-Getick) This PR was merged into the 3.2-dev branch. Discussion ---------- [HttpFoundation] Deprecate extending some methods | Q | A | ------------- | --- | Branch? | "master" | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony/symfony#19727 | License | MIT | Doc PR | It's really hard to change methods signature because of bc. I'm proposing to deprecate extending some getters/setters of `Response` because of this (and because extending them is not really useful). If you like this approach it could be used in other places to simplify bc in 4.0. Edit: This causes issues (warnings always triggered) when mocking `Response` entirely but does it matter as people should only mock needed methods? Commits ------- c0a26bc [HttpFoundation] Deprecate extending some methods
Backwards compatibility is assured by typehinting for DateTimeInterface, and converting to \DateTime for onward use
This is my first PR for Symfony, so please let me know if I've done something not quite right.