-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Deprecate ResourceInterface::getResource() #15719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate ResourceInterface::getResource() #15719
Conversation
I think we need to introduce a whole new resource system, which would move the checking of freshness outside the resource (it is not possible to implement such feature in the existing interface). As such, I don't think we need to deprecate part of the interface now (we should deprecate the whole interface when implementing the new system) |
@mpdude the issue is that |
@stof For resources implementing If we remove So, we could extract that into a base interface ( |
@mpdude a resource should also be serializable, as we serialize it in the meta file. This is why I think we need a new interface (freeing us from BC on the interface itself), with an implementation wrapping the existing ResourceInterface. |
Is it really necessary to extend \Serializable? What difference does it make? |
@mpdude makig sure resources are actually serializable (people broke things in the past when putting a PDO instance in their own resources as this would not make them serializable anymore) |
Yes, but they will find out rather quickly and still can implement \Serializable, not? For most simple resources that work fine with the built-in serialization mechanism, I'd like to avoid the hassle of having to implement __sleep and __wakeup. Plus it's not a BC issue on |
@mpdude if you don't implement Serializable, it also means you are not allowed to modify your class anymore, because it would break BC for the unserialization of existing meta. This is why core resources are serializable btw (and also to reduce the size of the serialized data) |
ping @fabpot / @symfony/deciders |
needs a rebase |
The return value of this method does not make sense if you do not exactly know about the type of resource at hand. For example, it may be [an array](https://github.com/symfony/symfony/blob/b49fa129bdb3c0aa970a006b16dd1ca63a9d7ebd/src/Symfony/Component/HttpKernel/Config/EnvParametersResource.php#L57) or a [file path](https://github.com/symfony/symfony/blob/87800ae47e64429f2544b798575d1cc1d4e5464a/src/Symfony/Component/Config/Resource/FileResource.php#L51). As all usages of getResource() within Symfony are in tests of particular Resource implementations anyway, deprecating and later removing this interface helps us with simplifying the ResourceInterface (#7176).
Rebased. |
Do we really need to keep this interface for 3.0? |
We need the __toString() for resource deduplication, plus there are |
Thank you @mpdude. |
This PR was merged into the 2.8 branch. Discussion ---------- Deprecate ResourceInterface::getResource() | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | n/a The return value of this method does not make sense if you do not exactly know about the type of resource at hand. For example, it may be [an array](https://github.com/symfony/symfony/blob/b49fa129bdb3c0aa970a006b16dd1ca63a9d7ebd/src/Symfony/Component/HttpKernel/Config/EnvParametersResource.php#L57) or a [file path](https://github.com/symfony/symfony/blob/87800ae47e64429f2544b798575d1cc1d4e5464a/src/Symfony/Component/Config/Resource/FileResource.php#L51). As all usages of getResource() within Symfony are in tests of particular Resource implementations anyway, deprecating and later removing this method helps us with simplifying the ResourceInterface (#7176). Commits ------- 87c0c7d Deprecate ResourceInterface::getResource()
…which was deprecated in 2.8 (mpdude) This PR was squashed before being merged into the 3.0-dev branch (closes #15929). Discussion ---------- [3.0][Config] Remove ResourceInterface::getResource() which was deprecated in 2.8 | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Deprecated in #15719. Commits ------- 7cef180 [3.0][Config] Remove ResourceInterface::getResource() which was deprecated in 2.8
The return value of this method does not make sense if you do not exactly know about the type of resource at hand. For example, it may be an array or a file path.
As all usages of getResource() within Symfony are in tests of particular Resource implementations anyway, deprecating and later removing this method helps us with simplifying the ResourceInterface (#7176).