orta / ARAnalytics

Simplify your iOS/Mac analytics
MIT License
1.84k stars 217 forks source link

Change min deployment target as Intercom and Localytics require #282

Closed delebedev closed 8 years ago

delebedev commented 8 years ago

That's a bold move and potentially a breaking change, however, keeping 7.0 has lead to some confusion already.

First of all, Localytics library referred in the podspec is already iOS 8 only. iOS 7 is supported with separate static framework: https://docs.localytics.com/dev/ios.html#getting-started-ios

Intercom goes further:

#if __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_8_0
#error This version (3.0.5) of Intercom for iOS supports iOS 8.0 upwards.
#endif

Their latest version won't even compile when used from within ARAnalytics.

DangerCI commented 8 years ago
:white_check_mark: Woo!
:white_check_mark:

Please include a CHANGELOG entry. You can find it at CHANGELOG.md.

Generated by :no_entry_sign: danger

BenchR267 commented 8 years ago

For me personally this is totally ok, but I'm not sure if we should do this only due to some confusion when working with 2 of the providers.. I think a library should have a deployment target as low as possible. If someone wants to use ARAnalytics just with Google Analytics in an iOS 7 project, this would not work. What do you say @orta?

orta commented 8 years ago

We might be able to do this per subspec?

delebedev commented 8 years ago

@orta I had no idea it is possible, it's definitely the way to go

orta commented 8 years ago

I'm low on time for today and OOO till friday - so give deployment subspecs it a shot, if it works 👍 - if it doesn't then I'l merge when we get back, now that iOS 10 is almost out, I'm ok with dropping 7 - but it will need a major SemVer bump.

BenchR267 commented 8 years ago

I have created a pull request (https://github.com/orta/ARAnalytics/pull/284) to increase the deployment target only for Localytics, it's working in my test project.

BenchR267 commented 8 years ago

Since https://github.com/orta/ARAnalytics/pull/284 is merged, this is no longer needed, so I'll close this.