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

Extracted Piwik::getJavascriptCode() into a separate, non static class #6446

Merged
merged 2 commits into from Oct 16, 2014

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Oct 15, 2014

Extracted Piwik::getJavascriptCode() into a separate, non static class: Piwik\Tracker\TrackerCodeGenerator.

The content of the methods hasn't been modified, I have just moved them.

Why?

  • clear up the almighty Piwik class which has too many responsibilities
  • a class with non-static methods means it's open to dependency injection
  • tests are in a separate class dedicated to this new class, so that's also simpler/cleaner

Piwik::getJavascriptCode() is now deprecated in favor of the new class (I have changed the places where it was called). I added the deprecation to the changelog.

I haven't renamed the Piwik.getJavascriptCode event for backward compatibility.

Feedback welcome.

@mnapoli mnapoli self-assigned this Oct 15, 2014
@czolnowski
Copy link
Contributor

Hi @mnapoli!
I was thinking about this method and way to refactor them. Clearly the first step is extraction into separate class. Great job! I think that next step might be extraction of all classes and helpers of tracking code into separate plugin. There are dependencies in CoreHome, SitesManager and Morpheus. All of them should be moved to separate plugin.

Clearing up almighty Piwik class is important! :+1

…ass: `Piwik\Tracker\TrackerCodeGenerator`

`Piwik::getJavascriptCode()` is now deprecated in favor of the new class.
@mnapoli mnapoli force-pushed the refactoring-tracker-code-generator branch from 2deb4cc to d9562a6 Compare October 15, 2014 21:07
@mnapoli
Copy link
Contributor Author

mnapoli commented Oct 15, 2014

As pointed by @tsteur it wasn't even an API method (Piwik\Plugins\SitesManager\API::getJavascriptTag() is the API method) so I removed the old method from the Piwik class, there is no need for deprecation. Even better!

Waiting for the tests to pass before merge.

mnapoli added a commit that referenced this pull request Oct 16, 2014
Extracted `Piwik::getJavascriptCode()` into a separate, non static class
@mnapoli mnapoli merged commit 00f37f0 into master Oct 16, 2014
@mnapoli mnapoli deleted the refactoring-tracker-code-generator branch October 16, 2014 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants