Remove the unused Ajax/Serializer.php class #8405
Conversation
|
@dmanners Sadly, we can not remove a class, since theoretically there might be an extension or customization which already relies on it in any possible way. |
|
@ishakhsuvarov fully get that you cannot remove an unused class in a minor release. So in this example of an unused class what is the path to removal? Deprecate and remove in future major release? |
|
@dmanners Currently our strategy on this case is to mark class as well as all of it's methods as deprecated. |
|
Cool @ishakhsuvarov then I will do that and replace Zend_Json while I am at it. |
dmanners
force-pushed
the
remove-zend-json-catalog
branch
from
89d79eb
to
31c041f
Compare
|
so @ishakhsuvarov I have added the deprecated noticed but I was also considering if we should call Any thoughts on that? |
| @@ -5,9 +5,31 @@ | |||
| */ | |||
| namespace Magento\Catalog\Block\Adminhtml\Product\Edit\Tab\Ajax; | |||
|
|
|||
| use Magento\Framework\View\Element\Template; | |||
|
|
|||
There was a problem hiding this comment.
Consider adding @deprecated annotation for class as well.
Also, a comment with motivation for deprecation is a really good thing.
|
@dmanners Currently the only thing we do is add annotations. |
|
@ishakhsuvarov I am still not sure what the best way to deal with deprecation is. Luckily in my day to day job I don't have to worry about people trusting my functions in that way as it is only me and my direct team working on them. I know symfony have some tools to help people like http://symfony.com/blog/paving-the-way-for-symfony-3-with-the-deprecation-detector-tool and I think they log deprecated calls also but I am not 100% about that currently. |
|
@dmanners |
| array $data = [] | ||
| ) { | ||
| parent::__construct($context, $data); | ||
| $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance() |
There was a problem hiding this comment.
- Please use
Magento\Framework\Serialize\Serializer\Jsoninstead ofSerializerInterface. - Since you are creating a new constructor, but not modify the existing one – you may use regular dependency injection technique.
dmanners
force-pushed
the
remove-zend-json-catalog
branch
from
a1ccbe3
to
4135880
Compare
|
@dmanners thank you for this PR. It successfully merged to develop branch |
Since this class is not used anywhere can we remove it as it is using Zend1 Json which is at end of life.