Fixed - renamed misleading method name deleteById to deleteBySku to … #4764
Conversation
…eflect actual behavior and added interface comment
|
Nice catch 👍 However, since maintainable version was released we cannot fix such sort of flaw this way (see http://devdocs.magento.com/guides/v2.0/architecture/backward-compatibility.html). Please add a new method |
…Sku to reflect actual behavior and added interface comment" This reverts commit 55c1d2d.
…ng method name deleteById as @deprecated.
|
Oh - I see =) like so? @see last commit. |
|
Yep, that's it! Looks good to me now, let's wait for Magento folks. |
|
Please always remember to add more info when deprecating. |
|
@adragus-inviqa thanks for the heads up =) Funny part is ... I actually wrote a deprecation notice at frist (since its mandatory in my dayjob), but looked@how it was previously done and removed my notice :p ... will do in the future. |
|
Why in the future? Why not now? People will ask themselves why is that deprecated. |
|
true =) done |
| * @throws \Magento\Framework\Exception\NoSuchEntityException | ||
| * @throws \Magento\Framework\Exception\StateException | ||
| */ | ||
| public function deleteBySku($sku); |
There was a problem hiding this comment.
Unfortunately this is still backward incompatible change as if exists other implementation of this interface it will be broken after new method addition.
There was a problem hiding this comment.
It does not matter. Unlike the method removal, it's feasible for 2.1, similar to situation described in #4749
|
@nimmneun please update your code with the latest develop |
|
@nimmneun Closing this PR for now due to inactivity. |
… reflect actual behavior and added interface comment. Updated corresponding webapi.xml + inconsistent whitespacing.
ProductRepositoryTest result: OK (34 tests, 177 assertions). Created dummy StoreExtensionInterface to run test (not commited).