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
Plugin updates are located within the \Updates namespace #10398
Conversation
We should not break API here I'd say as the updater is not so good. Eg it doesn't update all the core and the plugins at once and it may fail depending on what is updated first. It should be backwards compatible and break it with Piwik 4 so developers have more time to fix it and when development mode is enabled we can tell them to update the code. Otherwise breaks plugins like https://github.com/piwik/plugin-CustomDimensions/blob/master/Updates/0.1.2.php . If we break API here and development mode is enabled, we even have to check on every single UI request whether all the updates are defined correctly or not. Otherwise it's too risky that developers do not notice this. |
@tsteur I've added the backward compatibility layer. Tested the following use case:
-> both plugin are updated and migration SQL queries of each are executed. |
// as of Piwik 3.0.0, we expect plugin updates within an Updates\ namespace | ||
// but in Piwik 3.0.0 we stay backward compatible | ||
// this could be @deprecated in Piwik 4.0.0 | ||
if(class_exists($backwardCompatibleClass, false)) { |
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.
How does class_exists find the old class when autoload is disabled? Are they always first included in loadComponentsWithUpdateFile()
?
Can you add a test or something to remind us to remove this in Piwik 4.0 and to make sure it does not get removed earlier? Should be added to DeprecatedMethodsTest
and can be likely eg done by moving this BC code into a separate method.
To fix it properly it should probably also fix the filename. Fillename is eg |
This also needs to be mentioned in the migration guide as otherwise the plugin will fail in future versions. It also needs to explain that they cannot keep the plugin compatible with an older Piwik version as soon as they add the Updates and that they have to increase the required Piwik version in plugin.json |
You convinced me now this is not a good idea, so closing. |
generate:update
does not work (as Piwik core will try to load the wrong class name ie. withoutUpdates\
namespace)generate:update
command works with this pull request. plugin updates are located in the Updates\ namespace within the plugin namespace, so we read the correct namespace.follows up commit: fda46f1
refs matomo-org/developer-documentation#139
Notes:
Updates\
namespace. We should mark this in the developer changelog?