refs #6257 visitorUUID equal by default for each new Piwik.Tracker #6258
Conversation
|
Thanks for the pull request! This is a good change, it just needs a little improvement to make sure in the future the feature will always work: a new test. piwik.js is tested automatically on travis CI find more info in your piwik at: /tests/javascript/ Maybe you can add a test around this code: https://github.com/piwik/piwik/blob/master/tests/javascript/index.php#L2016 - Cheers |
mattab
added
Enhancement
|
Hey! Just commited unit tests for thist request. |
| * | ||
| * @return string Visitor ID in hexits (or null, if not yet known) | ||
| */ | ||
| getVisitorId: function () { |
There was a problem hiding this comment.
maybe remove this getter and simply inline the code in tests?
There was a problem hiding this comment.
well I believe that making getVisitorId available at Piwik level emphasis the idea of having same default visitorUUID across Trackers
There was a problem hiding this comment.
you may be right but your new implementation just makes sense so I don't think we need extra emphasis about how it works. in this case maybe it's better not to expose APIs.
There was a problem hiding this comment.
Agread changing API is not a best idea.
|
The tests look good! well done. And the build passes. let me know about the comment and i'll merge it! |
refs #6257 visitorUUID equal by default for each new Piwik.Tracker
|
looks good & well done! |
mattab
added
the
duplicate
No description provided.