search

Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
41 stars 12 forks source link

Time to cut version 1.0.0? #225

Closed mokagio closed 1 year ago

mokagio commented 1 year ago

While working on updating Sentry to version 7 and adding support for performance monitoring, I introduced a breaking change in the form of a new required property in CrashLoggingDataProvider.

This is no big deal. Tracks is an internal library with only a few clients and we control the update process for all of them. The library is also at version 0.x, and according to SemVer one can expect breaking changes when in this pre release state.

Still, it made me wonder if there's any reason why we haven't cut a 1.0.0 version of this library yet and whether it's a good time to do it.

What would it take to reach 1.0.0? Is it just a matter of bumping the major version value and calling it a day, or are there API signatures that we're unhappy with and should update first?

mokagio commented 1 year ago

cc @bjhomer as you've done lots of work with the library when adding SPM support.

leandroalonso commented 1 year ago

@mokagio one way to overcome this without affecting the current clients is to just provide a default value in a protocol extension — maybe if that's going to be adopted only by some apps might make sense. But the new property it's a minor change so I'm not so worried.

What worries me a bit is how Tracks is becoming more and more coupled to Sentry. I don't have enough context as to the decisions of why, but if we decide to change the crash tool in the future we're in for quite an adventure. But I'm deviating from your original question, so for me, it's ok to cut a 1.0.0.

bjhomer commented 1 year ago

are there API signatures that we're unhappy with and should update first?

I've actually been wanting to propose one API change related to A/B test variations. I filed it here.

mokagio commented 1 year ago

@leandroalonso:

What worries me a bit is how Tracks is becoming more and more coupled to Sentry. I don't have enough context as to the decisions of why, but if we decide to change the crash tool in the future we're in for quite an adventure. But I'm deviating from your original question, so for me, it's ok to cut a 1.0.0.

That's a good concern. We're trying to keep the implementation agnostic but of course with Sentry being our only crash reporting client, the our API shape kinda is somehow molded on theirs.

@bjhomer awesome, thanks for pointing that out. I don't see any rush in tagging a 1.0.0, so we should definitely iron out any possible breaking change before moving forward with it 👍

mokagio commented 1 year ago

one way to overcome this without affecting the current clients is to just provide a default value in a protocol extension — maybe if that's going to be adopted only by some apps might make sense.

Right! I actually didn't consider that option 😅 🤦‍♂️ I'm biased towards having verbose configurations, so there are no surprises. But ☝️ it would be super helpful to default to .disabled as an additional safety-net against accidentally enabling the feature and inadvertently blow up cost. Thanks!

mokagio commented 1 year ago

226 has been closed (thanks @bjhomer) and we should be ready to tag 1.0.0.

I haven't done it yet because I want to investigate what looks to me like incorrect flush behavior in WooCommerce. If there's a bug, I'd like to fix it before that.