search

matomo-org / matomo-sdk-ios

Matomo iOS, tvOS and macOS SDK: a Matomo tracker written in Swift
MIT License
390 stars 164 forks source link

Custom Variables in 4.x #204

Closed zantoku closed 6 years ago

zantoku commented 6 years ago

We have a "legacy" project that relies on the use of Custom Variables. We can't readily change to Custom Dimensions, so until now we've been forced to stay on the old 3.x version of the SDK.

However, we really want to move to the 4.x branch of the SDK so we don't run into a situation where the old SDK breaks for some reason and we're left unsupported.

So what I've done is add support for Custom Variables (and improve the ObjC-interop a little), largely following the implementation pattern for Custom Dimensions. Can we contribute the code to the project?

zantoku commented 6 years ago

If you want to review the code, I've created a pull request here: https://github.com/piwik/piwik-sdk-ios/pull/205

brototyp commented 6 years ago

Hi @zantoku, thanks for your information here and the pull request. I will check it out and see how it fits into the SDK. Generally I am not sure how deprecated features should be integrated into the SDK.

As far as I see there are a few options.

  1. Add them as first class citizens. (This is basically what you did)
  2. Add them as first class citizens but deprecate them right away. (And possibly define a version where it will become unavailable)
  3. Add them as second class citizens, deprecate them and offer them as a part of the core SDK. This could be achieved by adding convenience methods to define customTrackingParameters.
  4. Document a way how it is possible to achieve with the existing SDK (by using the customTrackingParameters for example) but not offering them as a part of the Core SDK.

I think 2 is better than 1, since it shows new users that there is a "better" way to achieve the same. 4 Could be a bit complicated for users that need the same features. What do you think?

zantoku commented 6 years ago

Hi @brototyp, seeing as how all the work for option 2 is already done, I'd appreciate if we could proceed with that! :) Do you want me to extend the pull request with deprecation annotations?

brototyp commented 6 years ago

Hi @zantoku, I am fine with option two. Can you add the deprecation annotations and a paragraph in the readme?

brototyp commented 6 years ago

This got implemented and merged in #223. It is be part of the 5.0.0-beta1 release.