[Fix] UrbanAirship-iOS-SDK mistakenly left off AudioToolbox and MessageUI frameworks #1318

Merged
merged 1 commit into from Feb 22, 2013

Conversation

Projects
None yet
3 participants

@dkuhnke To double check here: is there a reason AudioToolbox and MessageUI were not included your update to the 1.4.0 spec? If not then this should fix the undefined symbols I ran into.

@mattyohe mattyohe [Fix] UrbanAirship-iOS-SDK.podspec
Adds required frameworks that were left off the 1.3.7 to 1.4.0 update.
9548a18
Member

keith commented Feb 22, 2013

This libraries are not listed on the required libraries list in their readme? https://github.com/urbanairship/ios-library

@keithbsmiley Neither is MapKit and it's still included...

So, I looked into it and the podspec is including a UI folder which contains classes like UAPushSettingsSoundsViewController and UAPushSettingsTokenViewController
which require object files like AudioServicesPlayAlertSound (AudioToolbox) and MFMailComposeViewController (MessageUI).

I don't personally know what all of this is for, but it's mentioned on this page as needed for "sample UI".

Either way, the current podspec as written is broken without these frameworks. I'm tempted to merge now and later just exclude the UI portion, unless it's useful to someone to have it left in.

Thoughts?

Member

keith commented Feb 22, 2013

From what you're saying it sounds like it might be a better idea to exclude the sample UI files that are included?

I'm going to just merge for now so that the spec isn't left in a broken state.

As I said, I don't know what the purpose of the "sample UI" is. If it's just sample code, then clearly it can be excluded from the spec, and then the unneeded frameworks can be removed as well.

@mattyohe mattyohe added a commit that referenced this pull request Feb 22, 2013

@mattyohe mattyohe Merge pull request #1318 from mattyohe/master
[Fix] UrbanAirship-iOS-SDK mistakenly left off AudioToolbox and MessageUI frameworks
19df06c

@mattyohe mattyohe merged commit 19df06c into CocoaPods:master Feb 22, 2013

Member

keith commented Feb 22, 2013

I'm not sure I understand how this fixes anything that needs to be fixed? If those demo files shouldn't be included than that's something we should deal with. Adding frameworks just to try and fix a different issue isn't a good solution.

The podspec you merged –while a linting succeeds– fails to compile. I identified two frameworks that were in the previous podspec that were not included when 1.4.0 was created, of which adding them allows a project to compile.

I'm all for removing the demo code –if that's indeed what it is– and the linked frameworks required for that demo code. If you'd like to track that down be my guest.

Member

keith commented Feb 22, 2013

The issue here isn't fixing a broken spec it's fixing it by adding things until something works. If you had just gone ahead and said that the code using MessageUI and the code using AudioToolbox was not part of the demo projects but part of the standard included UI none of this would have been an issue.

@keithbsmiley Be reasonable.

I personally don't use things from the UI folder and clearly acknowledged this. I obviously wasn't "adding things until something works" as there was a previous podspec to work off of that worked fine, and I tested and linted my fix.

My issue here was fixing a broken spec, I have no idea what your issue is.

Member

keith commented Feb 22, 2013

Thanks for fixing the spec!

@mattyohe is it just me, or does this miss all of the shared resources like UAPushLocalization.bundle

@mattyohe mattyohe added a commit that referenced this pull request Jan 29, 2014

@mattyohe mattyohe Merge pull request #1318 from mattyohe/master
[Fix] UrbanAirship-iOS-SDK mistakenly left off AudioToolbox and MessageUI frameworks
626f1a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment