TelemetryDeck / SwiftSDK

Swift SDK for TelemetryDeck, a privacy-conscious analytics service for apps and websites.
https://telemetrydeck.com/
Other
155 stars 32 forks source link

Make it easier to check for appLanguage, preferredLanguage, and region + grand rename #153

Closed Jeehut closed 4 months ago

Jeehut commented 5 months ago

This is a follow-up on #149 with the branch renamed to grand-rename and the decisions from https://github.com/TelemetryDeck/docs/pull/85 applied. This includes the changes from #149:

In Swift, Locale.current represents the language and region the app is currently using. When an app is not localized to say German, for example, the identifier will not return de for the language even it's the users preferred language.

To make it easier for TelemetryDeck users to use a data-driven approach in deciding which languages to localize for next, it is important to add the preferred language of the user though, even if the app doesn't support it. It can also be useful to see just the app language or just the region, without them being clutched together in locale as in en_DE.

This PR therefore introduces region (the regional part of locale), appLanguage (the language part of locale), and the new preferredLanguage which is the field that is most useful to make the localization expansion decision.

In addition, I have renamed TelemetryManager.send("A", with: ["a": "b"]) to TelemetryDeck.signal("A", parameters: ["a": "b"]) as well as TelemetryManager.initialize to TelemetryDeck.initialize as per https://github.com/TelemetryDeck/docs/pull/85#discussion_r1589092178.

Jeehut commented 5 months ago

Please note that I found a for parameter and also a floatValue parameter on the send function. I renamed for to customUserID to be consistent with the configurations defaultUserIdentifier (which could be renamed to defaultUserID in the future). The floatValue didn't change. But I found that the order was like this:

  1. Signal Name (required)
  2. Custom User ID
  3. Float Value
  4. Parameters

It's more common to place parameters used more often at the front though and ones that are usually skipped to fall back to the default values at the end. So I changed the order as follows (basically switched 2 and 4):

  1. Signal Name (required)
  2. Parameters
  3. Float Value
  4. Custom User ID

To still provide fix-its for most cases, I added a copy of the function which only takes the Signal Name and Parameters and made sure it's favored over the function with more parameters if only 2 are provided. Otherwise, Xcode is unable to apply fix-its when the order of parameters change, but I think fix-its can be really useful to save time in migration.

Jeehut commented 4 months ago

I just updated this with a few bug fixes from my full setup testing. Biggest change is the addition of a new TelemetryDeck target which was needed so import TelemetryDeck works. I basically added an empty target which just imports the stuff from TelemetryClient so existing users with already-written code don’t get affected and everything continues to work.

winsmith commented 4 months ago

Seems like the linter is failing but from the error message it's not something introduced in this PR

Jeehut commented 4 months ago

@winsmith I think it's related to the upgrade to Swift 5.9 to properly support the visionOS platform in the Package manifest. That requires Xcode 15, and the CI has Xcode 14.

Jeehut commented 4 months ago

@winsmith I just adjusted the CI configuration and fixed all tests and more. Here's all I did:

Please take another look.