Fix #1779: CarrierInterface additional requirement #11822
Conversation
- Deprecated AbstractCarrierInterface - Removed references to AbstractCarrierInterface in fthe CarrierFactoryInterface, changed them to CarrierInterface. - Removed AbstractCarrier model implementing the AbstractCarrierInterface. - Changed references from AbstractCarrierInterface to CarrierInterface in de Config model.
magento-engcom-team
added
2.2.x
bugfix
Reproduced on 2.1.x
| @@ -17,7 +17,7 @@ | |||
| * @api | |||
| * @since 100.0.2 | |||
| */ | |||
| abstract class AbstractCarrier extends \Magento\Framework\DataObject implements AbstractCarrierInterface | |||
| abstract class AbstractCarrier extends \Magento\Framework\DataObject | |||
There was a problem hiding this comment.
Please, revert this change because it is backward incompatible.
| /** | ||
| * Interface AbstractCarrierInterface | ||
| * | ||
| * @deprecated | ||
| * @see CarrierInterface | ||
| */ | ||
| interface AbstractCarrierInterface |
There was a problem hiding this comment.
You must extend CarrierInterface here.
| @@ -5,7 +5,7 @@ | |||
| */ | |||
There was a problem hiding this comment.
Please, revert all these changes because they cause backward incompatible issue.
| @@ -8,7 +8,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please, revert all these changes because they cause backward incompatible issue.
| @@ -5,199 +5,12 @@ | |||
| */ | |||
There was a problem hiding this comment.
Leave methods isStateProvinceRequired, isCityRequired, debugData in this interface. These methods must be also deprecated.
There was a problem hiding this comment.
As described in this comment by @okorshenko the AbstractCarrierInterface should be empty
#11293 (comment)
There was a problem hiding this comment.
As described in comment that you mentioned "We need to move all the “correct” methods to CarrierInterface", so I please you to leave unused methods in deprecated interface. When deprecated interface will be deleted, unused methods also will be removed.
|
@dverkade Will you continue to work on this PR? |
…779_CarrierInterface_additional_requirement
- implement getAllowedMethods
|
Thanks for chaning this, I was not able to make time last week. I see this is now approved and will be processed further. |
|
Hi @dverkade, after internal discussion, we came to that fact that we need to introduce new interface that will have correct methods, for this we must make some HLD.So I will close this PR as current solution will be backward incompatible and will not bring any benefit. Thank you for your contribution. |
Fixed for issue #1779 as mentioned in closed PR #11293 :