search

matrix-org / matrix-analytics-events

Cross-platform definitions of analytics events raised by matrix SDKs
Apache License 2.0
8 stars 8 forks source link

Add a new platform code super property #104

Closed BillCarsonFr closed 5 months ago

BillCarsonFr commented 5 months ago

Fixes https://github.com/element-hq/crypto-internal/issues/316

Unfortunately I cannot just use the existing app platform, because it uses values with white space (Web Platform). The script we use to generate swift/kotlin enum will generate invalid code for such values. We are using some custom python script to convert, not sure if worth finding how to fix them

richvdh commented 5 months ago

I have to say that having two properties that do the same thing seems like a very bad idea. Can't we just fix the script?

At the very least, the comments should say how the new property differs from the old one, and which we should be using in future.

BillCarsonFr commented 5 months ago

I have to say that having two properties that do the same thing seems like a very bad idea. Can't we just fix the script?

At the very least, the comments should say how the new property differs from the old one, and which we should be using in future.

Yes fixing the script would be better. But given that the existing appPlatform is only used by web, I thought I might as well deprecate it and avoid the effort to change all the existing code generation script. On web it's also used in RS and maybe it was a matomo thing previously.

richvdh commented 5 months ago

Well, could you at least add a comment to appPlatform to say that it is deprecated?