search

AmpersandHQ / ampersand-magento2-upgrade-patch-helper

Helper script to aid upgrading magento 2 websites by detecting overrides. Now supports third party module detections
GNU Lesser General Public License v3.0
322 stars 39 forks source link

Report Plugins On Non-Existent Classes #99

Closed sprankhub closed 1 year ago

sprankhub commented 1 year ago

I just updated a project, where I missed quite some customisations. In the project, Klarna was heavily customised. We updated the Klarna module to a new major version, where quite some classes were moved. Hence, some plugins were defined on non-existent classes after the update. I think having plugins on non-existent classes is worth a warning in this module to catch those cases. What do you think?

convenient commented 1 year ago

@sprankhub https://github.com/AmpersandHQ/ampersand-magento2-upgrade-patch-helper/blob/34a19cd6597a51a7eca99bd87c47e9d850daf062/src/Ampersand/PatchHelper/Patchfile/Entry.php#L207-L214

So back in the day when i was putting this together i seem to think that plugins on non existant classes / methods were logged? 🤔

The current implementation of plugins really relies on the following approach

so i guess maybe we could change this code a little bit if the file had been removed to scan all the interceptable functions from BEFORE the change, and get a list of plugins. and if the file has been removed every one of those plugins needs to be reported.

sprankhub commented 1 year ago

I've never seen plugins on non-existent classes / methods being logged. Hence, I think this is a valid use case :) Also, "your plugin will become ineffective" is not a good argument IMHO, because this means you might miss a customisation, which you now need to implement differently.

convenient commented 1 year ago

oh yeah totally happy to be improved upon here. I just kind of had the impression that magento made noise about it somewhere, which it seems i was mistaken.