Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@alloy <= Use Frameworks #520

Merged
merged 22 commits into from
Jul 2, 2015
Merged

@alloy <= Use Frameworks #520

merged 22 commits into from
Jul 2, 2015

Conversation

orta
Copy link
Contributor

@orta orta commented Jun 15, 2015

I know what you're thinking, "but there's already an unmergable PR for frameworks." Well guess what, there's more. A bug in the de-duplicating version of means that you can't get away with #import "Pods.h" which meant that I had to ensure we didn't use any of these in our app ( which is what we should be doing anyway ) so I made this PR to give someone a roadmap for when frameworks gets in.

This PR totally breaks Facebook login, we really need that auth lib finished
This has been re-implmented with the new Facebook API, so it will need some manual testing.

@alloy
Copy link
Contributor

alloy commented Jun 15, 2015

Yay, more 🎉 😄

A bug in the de-duplicating version of means that you can't get away with #import "Pods.h" which meant that I had to ensure we didn't use any of these in our app ( which is what we should be doing anyway )

I’m not sure what this means. Is that a bug in CocoaPods? And what is it that we should have been using and what we shouldn’t have been using?

@orta
Copy link
Contributor Author

orta commented Jun 15, 2015

CocoaPods allows accessing Pods' header files individually like #import "HeaderInPods.h", which is wrong, it should be #import <Pod/HeaderInPod.h>. This PR is using marius' PR which briefly broke this "feature".

@alloy
Copy link
Contributor

alloy commented Jun 16, 2015

Gotcha, thanks 👍

@orta orta changed the title [WIP] = Frameworks 2 @alloy <= Use Frameworks Jul 1, 2015
@@ -344,33 +348,6 @@ - (void)showQuicksilver
[navigationController pushViewController:adminSettings animated:YES];
}

- (void)checkForiOS7Deprecation
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiling for iOS8 now, in theory we could do something similar for ios8 later next year

alloy added 4 commits July 1, 2015 14:24
This makes it so that OSS people will be able to build+run on the
simulator without any changes needed to the project (it is needed when
they’d want to run on device) and also means the CI server will simply
be able to run the tests without any changes.
@orta orta force-pushed the orta-frameworks2 branch 2 times, most recently from e2db64c to a89b7f6 Compare July 1, 2015 17:14
@orta
Copy link
Contributor Author

orta commented Jul 1, 2015

Giphy

@orta
Copy link
Contributor Author

orta commented Jul 1, 2015

ready for review

@alloy
Copy link
Contributor

alloy commented Jul 2, 2015

This has been re-implmented with the new Facebook API, so it will need some manual testing.

Has this been done?

And what about on 📱 ?

@alloy
Copy link
Contributor

alloy commented Jul 2, 2015

Otherwise the patch looks good to me 👍

@orta
Copy link
Contributor Author

orta commented Jul 2, 2015

And what about on 📱 ?

Doing it on an iPad ATM in-between eidolon travis runs. 👍

@orta
Copy link
Contributor Author

orta commented Jul 2, 2015

Confirmed. I can log in to my account via Facebook when running on an iPad in store config 👍

@orta
Copy link
Contributor Author

orta commented Jul 2, 2015

Alright, I'm doing it.

MRW I want to hop on an old bandwagon but I don't have any fancy software

orta added a commit that referenced this pull request Jul 2, 2015
@orta orta merged commit c632235 into master Jul 2, 2015
@alloy
Copy link
Contributor

alloy commented Jul 2, 2015

Awesome 👍

@orta orta modified the milestone: Sprint June 2 - July 14 Jul 3, 2015
@alloy alloy deleted the orta-frameworks2 branch September 2, 2015 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants