search

segment-integrations / analytics-ios-integration-amplitude

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

Version dependencies in Podfile are too restrictive #72

Closed ronaldsmartin closed 4 years ago

ronaldsmartin commented 4 years ago

Hello,

By specifying the patch version in your Podfile dependencies, you're preventing library consumers from updating beyond the minor versions you specify here (docs).

https://github.com/segment-integrations/analytics-ios-integration-amplitude/blob/92ba4ff132411581a3496735c8cad21fca7fd3cb/Segment-Amplitude.podspec#L24-L25

These should just be ~> 3.7 and ~> 4.8, respectively.

ndonald2 commented 4 years ago

I will second this. This is too restrictive. Updating from Segment-Amplitude 2.x to 3.x actually results in a regression of the versions of these secondary dependencies.

ronaldsmartin commented 4 years ago

I have since moved companies and am no longer in contact with Segment, but I actually reached out to Segment Support at the time and was told that this was intentional on their part. Going to leave this issue and the PR open because it does block people, as you say. 🤷

CC @benasher44

ndonald2 commented 4 years ago

Tagging other contributors - @ccnixon @bsneed @ladanazita - any chance this can be fixed? This is preventing us from updating the core Segment SDK. Is the amplitude integration not compatible with later versions?

bsneed commented 4 years ago

@ndonald2 currently, Analytics is unpinned now, which we would like to keep that way. We haven't specifically tested with later versions. I'll look and see what needs to happen to be able to use the latest. Sound good?

ndonald2 commented 4 years ago

Hi @bsneed, thanks for the follow up.

Analytics is unpinned now

That does not appear to be true according to the current podspec in this repo. It is pinned to ~> 3.7.0 which per the Cocoapods syntax means the dependency will not be satisfied by 3.8 or any later versions. Latest version is 4.0.

https://github.com/segment-integrations/analytics-ios-integration-amplitude/blob/92ba4ff132411581a3496735c8cad21fca7fd3cb/Segment-Amplitude.podspec#L24-L25

Currently, if I update my local podfile to pod "Analytics", "~> 4.0" then pod install fails with an unsatisfied dependency since the segment amplitude integration requires an Analytics version at 3.7.x.

Happy to provide any other details necessary, but otherwise sounds great if you all are looking at how the latest version might be supported.

bsneed commented 4 years ago

@ndonald2 no problem! are we talking about the same thing?

https://github.com/segment-integrations/analytics-ios-integration-amplitude/blob/master/Segment-Amplitude.podspec

ndonald2 commented 4 years ago

Ah! Thanks so much for pointing that out! I see that commit now - did you just merge that to master? I could be totally mistaken but I just copied the line links from master and didn't see that in there...

In any case the trunk Podspecs repo is not showing 3.0.2 as available yet so I think once that propagates this will fix the issue.

$ pod repo update
$ pod search Segment-Amplitude

-> Segment-Amplitude (3.0.1)
   Amplitude Integration for Segment's analytics-ios library.
   pod 'Segment-Amplitude', '~> 3.0.1'
   - Homepage: http://segment.com/
   - Source:   https://github.com/segment-integrations/analytics-ios-integration-amplitude.git
   - Versions: 3.0.1, 3.0.0, 2.0.1-beta, 2.0.0, 1.5.0, 1.4.4, 1.4.3, 1.4.2, 1.4.1, 1.4.0, 1.3.0, 1.2.0, 1.1.1,
   1.1.0, 1.0.0, 1.0.0-alpha [trunk repo]
bsneed commented 4 years ago

@ndonald2 ah, I see what happened. I didn't have access to release 3.0.2 at the time, so it didn't get published to cocoapods. In the meantime, use master from your pod file and you should be fine if that's all it was. Should I still check out upgrading the Amplitude lib?

ndonald2 commented 4 years ago

Great, thanks!

Should I still check out upgrading the Amplitude lib?

It's not an immediate issue because this plugin is compatible with the latest Amplitude lib release, but if at any point they bump up a minor version to 4.9.0 then the same issue will manifest with unsatisfied dependency. As long as the supported version of Amplitude doesn't fall too far behind in the future it's not a big deal.

Thanks so much!

bsneed commented 4 years ago

@ndonald2 I removed the version pin of Ampltiude as well as updated to their new pod Amplitude which is at 5.2.1. Lemme know how it goes! 🙇‍♂️

edit: I also pushed up the 3.1.0 version of Segment-Amplitude pod.