[Vue] migrate all directives to vue

matomo-org / plugin-AnonymousPiwikUsageMeasurement

Help making Matomo better by sending anonymized usage data to the creators the project, and/or to your own Matomo instance and/or to any other Matomo instance

https://plugins.matomo.org/AnonymousPiwikUsageMeasurement

GNU General Public License v3.0

4 stars 12 forks source link

[Vue] migrate all directives to vue #69

Closed diosmosis closed 2 years ago

diosmosis commented 2 years ago

Description:

Changes:

migrated each tracking directive to vue w/ an angularjs adapter.

Review

[ ] Functional review done
[ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
[ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
[ ] Security review done
[ ] Wording review done
[ ] Code review done
[ ] Tests were added if useful/possible
[ ] Reviewed for breaking changes
[ ] Developer changelog updated if needed
[ ] Documentation added if needed
[ ] Existing documentation updated if needed

sgiehl commented 2 years ago

@diosmosis I did not yet have a closer look, but guess for plugins, not included in core, we will need to increase the Matomo requirement in plugin.json, right?

diosmosis commented 2 years ago

@sgiehl yes. To my understanding these will be done last. Since AnonymousPiwikUsageMeasurement is a submodule it is included in core, though, correct?

sgiehl commented 2 years ago

No. Only tag manager is shipped with core. The other plugins can only be installed through the market place

diosmosis commented 2 years ago

@sgiehl I see, I'll mark it as a draft for now.

sgiehl commented 2 years ago

@tsteur how shall we handle the migration of such plugins? Is it fine to bump the version requirement to Matomo 4.9 and release a new version after the plugin was fully migrated? Maybe it makes sense to wait till the core migration is finished, to ensure no adjustments in the plugin will be needed though.

tsteur commented 2 years ago

I don't have a big preference @sgiehl for the free plugins I'm happy to bump the required version number if it simplifies things and if it reduces the chances of breaking too many things . When would the core migrations be finished roughly?

diosmosis commented 2 years ago

@tsteur prs will be up in a week or two. Not sure how fast the reviews will go.

diosmosis commented 2 years ago

@sgiehl @tsteur bumped the minimum required matomo version. I'm thinking its fine for plugins like this since they should rarely be updated and new updates can go into a branch.

diosmosis commented 2 years ago

Also I'm not entirely sure who would be reviewing these PRs, I'll be assuming @sgiehl / @AltamashShaikh but please correct if that is not right.

tsteur commented 2 years ago

@diosmosis yes for free plugins it should be no problem to bump version number from my perspective

justinvelluppillai commented 2 years ago

Also I'm not entirely sure who would be reviewing these PRs, I'll be assuming @sgiehl / @AltamashShaikh but please correct if that is not right.

Yes @AltamashShaikh will normally review them, and he will involve @sgiehl or me if needed.

AltamashShaikh commented 2 years ago

@diosmosis / @sgiehl / @tsteur Do you guys know how to test for multisite ?

diosmosis commented 2 years ago

Hi @AltamashShaikh I looked at the code again and I think its better to wait before reviewing this, there will be more changes merged that will require changes here so better to just wait. Apologies if it took a lot of time.

diosmosis commented 2 years ago

@AltamashShaikh those events require 4.x-dev to be up to date, can you pull and clear your cache? Apologies I should have mentioned when I pinged you.

AltamashShaikh commented 2 years ago

@diosmosis do we need to update the min core version here too ?

diosmosis commented 2 years ago

@AltamashShaikh :+1: bumped the version