-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Remove deprecation without alternative #11070
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
Conversation
I believe they were deprecated with some existing implementation in mind. Isn't |
@schmengler you are right that In this particular case, CartInterface was replaced with the |
0c0393d
to
e442de5
Compare
@vrann that would be better than the status quo but after I talked to @antonkril yesterday, I'd like to suggest a third way: Add a comment "Will be deprecated", as in The reason is, that the That leaves us with only one reasonable way to add products to the cart, namely |
@schmengler that sounds like a reasonable compromise. Please re-create your PR with the new changes. |
Do we really need to un-deprecate it literally instead of adding comment regarding partially suiable interfaces + the note in some scenarios you still have to use this deprecated class? Like |
Was the question for me? I don't see any issues with it :)
IMHO, yes. If you find methods that have a feasible alternative, you could mark them as deprecated individually. But the whole class, no. Meanwhile it's even covered with |
Currently PR lacks this information, maybe some other interfaces are also suitable. As to me class and interface must remain deprecated but contain maximum information regarding alternatives. Let's wait for feedback from @vrann. |
@schmengler Seems |
@vkublytskyi you're right, that's a good alternative. It's slightly less convenient because you cannot just pass product id but that's okay, this makes it a more type safe interface. Unfortunately it is not part of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Quote/Api/Data/CartInterface.php is the only interface implemented by quote model and it is pretty useless here.
Description
The
Cart
andCartInterface
have been deprecated a while ago without ever providing useful alternatives, causing much confusion amongst developers.Later, deprecation guidelines were introduced:
We should update deprecation annotations accordingly and "un-deprecate" classes and methods that do not have any alternatives yet.
Fixed Issues (if relevant)
Contribution checklist