search

keenlabs / KeenClient-iOS

Official iOS client for the Keen IO API. Build analytics features directly into your iOS apps.
https://keen.io/docs
MIT License
78 stars 56 forks source link

Add proxy support #176

Closed heitortsergent closed 7 years ago

heitortsergent commented 8 years ago

This PR closes #171.

This change will require users to add CFNetwork.framework to their binaries... :/ I'm trying to find a way to make this optional.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.4%) to 60.777% when pulling 4403e0d7dbd89348df30bf15039296c6f2fd6bb9 on add_proxy_support into 7cecd2cc3d6866eb6563d5fb8e548c4e3f976e0b on master.

heitortsergent commented 8 years ago

@terryhorner thanks for the comments! Made the changes, I went back to the previous init implementation, just added a if (self) check to it.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.4%) to 60.847% when pulling 74b7aff3da2d29727c5e7ead633389b799ec1969 on add_proxy_support into 7cecd2cc3d6866eb6563d5fb8e548c4e3f976e0b on master.

terrhorn commented 8 years ago

👍

heitortsergent commented 8 years ago

@terryhorner finding a way to make it optional is taking longer than I expected. :/

In the meantime, do you think we could merge this PR and also add a 'beta' release tag to it, so users could test it out? (and can you approve the review please? :shipit:)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.4%) to 60.847% when pulling 3aafab62072f3a5d234d9500c6fcf3f640bb7d9a on add_proxy_support into 59250d0bacfd655e0b7df4ddc29434c02e266070 on master.

terrhorn commented 8 years ago

I am hesitant to approve a beta release due to the fact that it could find its way into apps that get pushed to the App Store only to never be updated in the future.

Is it possible to make CFNetwork.framework optional? If not, we'll have to deal with it as is.

heitortsergent commented 8 years ago

@terryhorner that's a good point...

The only working possibility I've found so far requires the users to add CFNetwork to their project build settings, and there they can mark it as optional. But that will break the build of everyone who updates the library, and the error Xcode throws is not really clear.

I'm trying to find if there are any alternatives methods that don't break existing builds.

baumatron commented 7 years ago

227 replaces this