segment-integrations / analytics-ios-integration-amplitude

The Amplitude analytics-ios integration.
MIT License
11 stars 45 forks source link

Specify the version Amplitude to depend on #80

Closed christianYoopies closed 4 years ago

christianYoopies commented 4 years ago

Current merge is supposed to resolve the problem of non-building the project after making

pod update/carthage update

due to major update of 'Amplitude'.

Thank you in advance for reviewing,

Regards

christianYoopies commented 4 years ago

Merging to master and further creating of a tag would highly appreciated

christianYoopies commented 4 years ago

@migs647 @bsneed @briemcnally

bsneed commented 4 years ago

@christianYoopies thanks! We don't want to pin analytics-ios versions, but if you can pull that bit out, I can merge this and put an update out. Thanks so much for the fix!

christianYoopies commented 4 years ago

@bsneed No problem, I cleaned up the Pull Request so it now only deals with Amplitude. Thank you:) I will be notified when you will merge and tag;)

bsneed commented 4 years ago

Thanks @christianYoopies ! Release getting pushed out shortly.

christianYoopies commented 4 years ago

Great, thank you for this @bsneed

bsneed commented 4 years ago

Hi @christianYoopies, we ended up putting a hold on this internally. For the meantime, you can point your pod for this to the master branch here. We need to do the updates to bring it to Amplitude 6.0 for iOS 14 / IDFA changes.

christianYoopies commented 4 years ago

@bsneed Thank your for your answer. The thing is that we are referencing to Amplitude in the .podspec file for now and unfortunately there is now other way but to link to a version in it because of cocoa pods restriction(you cannot to specify a concrete branch in .podspec file syntax). We are not in a big hurry but having some tag(even a patch tag) in couple of weeks would be very nice for us so the team would have been able to do 'pod update':)

Thank you again, Regards

bsneed commented 4 years ago

@christianYoopies What we're going to do is issue a 3.2.1 update with your changes, and roll the amplitude-ios 6.0 support into a 4.0.0 release. Thanks for your patience on the back and forth with this. I'll issue a release in just a few minutes. Quick question though, is there any reason to use 5.2.1 over 5.3.0?

christianYoopies commented 4 years ago

@bsneed Well I believe there is no vicious reason to say 5.2.1 over 5.3.0 from what I see in the changelog of version 5.3.0. ~> 5.2.1 will get 5.3.0 for all the users finally. I did put 5.2.1 because 5.2.1 was in our Podfile.lock here locally:) I can do another PR with ~> 5.3.0 if you wish, or you can do the update directly to master for the things to go faster as it is a tiny change ;)

bsneed commented 4 years ago

@christianYoopies ok, gotcha. One additional question. We pulled amplitude-ios 6.0 and didn't see any incompatibilities there, just that they stopped collecting IDFA being the only change. Can you elaborate on what you're seeing re Carthage there?

christianYoopies commented 4 years ago

@bsneed I will check carefully and let you know asap

christianYoopies commented 3 years ago

@bsneed Segment_build_error

This is an error we have, sorry for late response