Add PHP 8.0 attributes support #707
Conversation
|
Seems good to me |
|
@derrabus Do you have time to review this PR? |
src/Configuration/Cache.php
Outdated
| $mustRevalidate = false, | ||
| $vary = [], | ||
| $lastModified = null, | ||
| $Etag = null, |
There was a problem hiding this comment.
I'm agree with you but i'm annoyed because on the documentation, we suggest to use "Etag" and not "etag" for the annotation Cache.
So, I can transform Etag to etag, but I have to add a information on the documentation to inform that for attributes, we use etag and not Etag. What do you think about that ?
|
Thanks for your review @derrabus, I'll see that |
Yes, I'll add more tests this week-end ;) |
|
Can you rebase on current master (that should help for the tests) |
|
Hi, |
|
Hi @nlhommet ! |
|
Thanks @Yoann-TYT for working on this feature, this is much appreciated. |
This PR was merged into the 6.1.x-dev branch. Discussion ---------- Restore undetermined state in Cache configuration This PR fix a BC break introduced by #707. Given the following code: ``` /** * @Cache(smaxage="+1min") */ public function fooAction(){} ``` The headers generated by `HttpCacheListener` changed as following: ```diff - public, s-maxage=60 + private, s-maxage=60 ``` Why? Because previously the Cache's properties were null until the user provide a value. In such state, `isPublic` returns false (although `isPrivate`). And sometimes, null is expected (like the `vary` array => https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/b3596907c3d35bee5a9a1096baaeb7d465a04f30/src/EventListener/HttpCacheListener.php#L135) The #707 PR introduce 2 changes: - a value for each property is now forced by the constructor's default value. - the constructor calls the setter for each property (even if the user did not provide a value for it), some setter (ie. `setPublic`) does not expect a null value and convert null into something. It looks like, only the Cache configuration is affected. Other Configuration has already a default value defined for each property. This PR restore the default `null` value for all properties and prevent the constructor to call the setter when the value is null. Commits ------- 99e749b Restore undetermined state in Cache configuration
| */ | ||
| public function __construct( | ||
| $data = [], | ||
| string $message = null, |
There was a problem hiding this comment.
This is a BC break, I opened #733 for more info
Add PHP 8.0 attributes support
Using PHP attributes if we start a new project on PHP 8.0
Instead of using classic doctrine annotations
We should use attributes PHP 8.0
Note for Etag attribute
In the documentation, we use Etag ( not etag or ETag used in the unit tests ). So, to simplify the code, I suggest to fix the unit test. What do you think about that ?
Thank you for your reading :)