Skip to content
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

Renames CustomPiwikJs plugin to CustomJsTracker #15505

Merged
merged 11 commits into from Feb 11, 2020
Merged

Renames CustomPiwikJs plugin to CustomJsTracker #15505

merged 11 commits into from Feb 11, 2020

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Feb 3, 2020

fixes #13604

@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 3, 2020
@sgiehl sgiehl added this to the 4.0.0 milestone Feb 3, 2020
@sgiehl sgiehl force-pushed the renamecustomjs branch 5 times, most recently from 469afe3 to 21efdb2 Compare February 3, 2020 15:39
@sgiehl sgiehl marked this pull request as ready for review February 3, 2020 15:48
@sgiehl sgiehl added the Needs Review PRs that need a code review label Feb 3, 2020
@tsteur
Copy link
Member

tsteur commented Feb 3, 2020

@sgiehl there used to be a plugin on the marketplace named CustomTrackerJs... just wondering if someone updates from Piwik 2.X to Matomo 4.X if this could cause any issues? I suppose that plugin would be simply overwritten? (which be kind of good since it would also automatically remove an outdated / not compatbile plugin) or is easier to rename it to CustomJsTracker?

@sgiehl
Copy link
Member Author

sgiehl commented Feb 4, 2020

Guess it would be overwritten, but not sure if some old files might remain 🤷‍♂
Not having a preference here. Can also rename it...

@tsteur
Copy link
Member

tsteur commented Feb 4, 2020

Up to you. If we keep the name, might be worth testing what happens when upgrading from Piwik 2 and such a plugin existed. Might be easier to maybe rename it?

@sgiehl
Copy link
Member Author

sgiehl commented Feb 5, 2020

renamed it again. Need to update the UI tests again, I guess. But should be ready for another review

Copy link
Member

@tsteur tsteur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few minor comments, nothing major. Mostly around docs.

Adjusted already 6 or so internal plugins. Be good to make the changes in tag manager and resolve the open issues then be good to merge

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
core/Updates/4.0.0-b1.php Outdated Show resolved Hide resolved
core/Updates/4.0.0-b1.php Outdated Show resolved Hide resolved
@sgiehl sgiehl force-pushed the renamecustomjs branch 2 times, most recently from 3eafb10 to af1f06d Compare February 7, 2020 07:59
@sgiehl
Copy link
Member Author

sgiehl commented Feb 7, 2020

@tsteur applied the feedback

@tsteur
Copy link
Member

tsteur commented Feb 9, 2020

Looks good in general @sgiehl some tests will need fixing though

@sgiehl
Copy link
Member Author

sgiehl commented Feb 10, 2020

Guess tests are failing because the changes in tagmanager are needed. will push a submodule change to prove that.

@sgiehl
Copy link
Member Author

sgiehl commented Feb 10, 2020

Tests seem to pass again. Before merging we need to merge matomo-org/tag-manager#220 and update the submodule ref here.

@sgiehl sgiehl changed the title Renames CustomPiwikJs plugin to CustomTrackerJs Renames CustomPiwikJs plugin to CustomJsTracker Feb 11, 2020
@sgiehl sgiehl merged commit ab1e700 into 4.x-dev Feb 11, 2020
@sgiehl sgiehl deleted the renamecustomjs branch February 11, 2020 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename CustomPiwikJs plugin to CustomMatomoJs (or CustomTrackerJs)
2 participants