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

Changes for usage of DeviceDetector 3.0 #7068

Merged
merged 21 commits into from Feb 2, 2015
Merged

Changes for usage of DeviceDetector 3.0 #7068

merged 21 commits into from Feb 2, 2015

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jan 24, 2015

This PR aims to enable Piwik to use the new DeviceDetector 3.0, which will be released around 1st of February.

DeviceDetector 3.0 has some bigger changes:

  • Caching was completely replaced
  • new device type (portable media player)
  • detection of windows was corrected

As part of the PR I've also adjusted the operating system information available for a visit, to match those available for browsers.

Todos

before merge:

  • set version in composer.json to ~3.0 as soon as it was released

after merge:

  • update screenshot tests - device type widgets will change due to new device type

@sgiehl sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Jan 24, 2015
@sgiehl sgiehl added this to the Piwik 2.11.0 milestone Feb 1, 2015
@tsteur
Copy link
Member

tsteur commented Feb 1, 2015

Looks good to me!

I noticed there is a breaking change in Live API because of renaming operatingSystemCode => operatingSystemName? Ideally we would deprecate it (having both properties for eg 2 months then remove the old one) but it might be ok in this case. It's up to you to deprecate or break it. Some other applications might depend on this property eg it could break Piwik Mobile if it would use that property https://github.com/piwik/piwik-mobile-2/blob/2.2.0/app/controllers/visitor.js#L262 (which it doesn't as we output OperatingSystem and BrowserName). There's a good chance other application use this property.

Do you mind documenting the changes in CHANGELOG.md?

  • The mentioned change either in "Breaking Changes" or "Deprecations"
  • The update to 3.0 in "Library updates". It shouldn't matter as it is not a public API but it still good to document it just in case. Eg PRO plugins sometimes access libs directly (which they should not) but they don't in this case as I had a look at the repos
  • I think there are new properties in the Live API output such as operatingSystemVersion. This could go into "New features" or "New APIs". We do not really have a good category for this. Maybe we need a new one or a different structure? Reckon "New APIs" would be best otherwise

@sgiehl
Copy link
Member Author

sgiehl commented Feb 2, 2015

operatingSystemCode still exists. I've added operatingSystemName and operatingSystemVersion so there are the same properties as for browsers. So there shouldn't be a problem.
I'll adjust the changelog later.

@tsteur
Copy link
Member

tsteur commented Feb 2, 2015

Sorry now I see it, only the order changed :)

@sgiehl
Copy link
Member Author

sgiehl commented Feb 2, 2015

@tsteur I've updated the changelog. If everything is fine, feel free to merge

@sgiehl sgiehl removed the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Feb 2, 2015
tsteur added a commit that referenced this pull request Feb 2, 2015
Changes for usage of DeviceDetector 3.0
@tsteur tsteur merged commit 09e8c41 into master Feb 2, 2015
@tsteur tsteur deleted the device-detector-3 branch February 2, 2015 21:35
@tsteur tsteur added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Feb 2, 2015
@mattab
Copy link
Member

mattab commented Feb 9, 2015

Nice to see the latest and greatest device detector 3.0.0 in Piwik!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants