@sgiehl opened this Pull Request on February 17th 2020 Member

refs #8567

@tsteur commented on February 17th 2020 Member

BTW: I'm wondering in that case it it's worth removing the class? Not sure there is much of a down side to keep it and at the same time it might force heaps of plugins to change the code maybe? Might be the same for some other classes /methods. Wondering if sometimes we could just keep the old classes for simplicity of BC. In this case it wouldn't hurt us much maybe to keep that old class?

Of course we'd still ideally change some usages to be a good example and make use of it correctly ideally.

In general since we manage > 50 plugins it might be easier to sometimes just keep the old classes if they are as simple as the translate class

@sgiehl commented on February 17th 2020 Member

We already deprecated that class in Piwik 2.11 and kept it in 3.x for BC. At some point we need to break BC and remove such stuff otherwise we will mess up the code with more and more such stuff :man_shrugging:

@tsteur commented on February 17th 2020 Member

At some point we need to break BC and remove such stuff otherwise we will mess up the code with more and more such stuff 🤷‍♂

Not sure how one file would mess up the code there? Of course it always depends but in this case not seeing it so much. There doesn't seem to be much of a harm as it's only forwarding methods anyway. If it helps keeping plugins easily compatible and less pain for us and developers be great. Personally, I love eg how Android and iOS etc keep BC basically almost forever (not saying we should always do this but when it's easy like there might be worth it)

@sgiehl commented on February 17th 2020 Member

OK. What about keeping that class but triggering a deprecation notice
whenever a method is called and remove it in Matomo 5 then?

@tsteur commented on February 17th 2020 Member

Not sure. I reckon in that case there might not even be a need to remove it in Matomo 5 either. I'm thinking if we can keep BC whenever it's easy for us we probably should. Could even remove the deprecated flag there. In development mode we could trigger a notice though maybe and recommend using a different way. 👍

@sgiehl commented on February 18th 2020 Member

@tsteur readded the file, triggering deprecation notices in development mode

@tsteur commented on February 18th 2020 Member

👍

@sgiehl commented on February 19th 2020 Member

@tsteur let me know if you think it's ready to merge. Would do that in all linked repos then and update the submodules before merge.

@tsteur commented on February 19th 2020 Member

@sgiehl I haven't really looked (only very quickly) but reckon it's fine 👍 Unfortunately won't be able to do any reviews in the next 7 days :(

This Pull Request was closed on February 20th 2020
Powered by GitHub Issue Mirror