Skip to content

Removing Pods directory from project. - #316

Closed
sendhil wants to merge 2 commits into
developfrom
issues/288
Closed

Removing Pods directory from project.#316
sendhil wants to merge 2 commits into
developfrom
issues/288

Conversation

@sendhil

@sendhil sendhil commented Nov 5, 2013

Copy link
Copy Markdown
Contributor

Fixes #288. h/t @xtreme-rebecca-putinski

@koke

koke commented Nov 5, 2013

Copy link
Copy Markdown
Member

Can you add some instructions to the README file? We'll need to tweak the build server as well

@sendhil

sendhil commented Nov 5, 2013

Copy link
Copy Markdown
Contributor Author

Sure.

@astralbodies

Copy link
Copy Markdown
Contributor

As a side note - Cocoapods doesn't have a great way to integrate yet with Xcode Bots. I'm not terribly surprised :)

@pivotal-rebecca

Copy link
Copy Markdown

There are instructions for reference in the refactor branch, when we removed the Pods there: https://github.com/wordpress-mobile/WordPress-iOS/blob/31b4172afb7772894d19a4e85226a82d4f0b1ffd/README.md

@pivotal-rebecca

Copy link
Copy Markdown

Has the project been built after removing the pods, then pod install?

I think with 'AFNetworking', '~> 1.3' you'll get 1.3.3 now which changes the authentication methods a bit. In Pods/WordPressApi/WPXMLRPCClient.m you'll get build errors since two of the methods cease to exist. The authentication block can be replaced with setWillSendRequestForAuthenticationChallengeBlock:block I believe, but not 100% on what to do about the protection space.

https://github.com/AFNetworking/AFNetworking/blob/9fd6fea0664627a45927475be2dedd3aaf7ddfb3/CHANGES#L112

cc @koke @sendhil

This was discovered on updating the pods for work on AFIncrementalStore integration.

@koke

koke commented Nov 5, 2013

Copy link
Copy Markdown
Member

I remember pinning pods to a specific version, but I don't know in which branch. We should make sure the pods stay in the current version

@pivotal-rebecca

Copy link
Copy Markdown

The Podfile.lock is supposed to do that, so hopefully it does successfully :)

DTCoreText also has updates which breaks colorWithHexString: which is replaced with DTColor *DTColorCreateWithHexString(NSString *hexString);

Might be a good idea to update some pods eventually though.

I'm using AFNetworking 1.3.3 for AFIncrementalStore, so I'll at least be updating that at some point (previous comment)

@sendhil

sendhil commented Nov 5, 2013

Copy link
Copy Markdown
Contributor Author

@xtreme-rebecca-putinski - I did test the build with pod install after this check in and it worked good.

@sendhil

sendhil commented Nov 6, 2013

Copy link
Copy Markdown
Contributor Author

Added the README, @beaucollins helped me out and got the build server working so that Jenkins will compile with the new changes.

@koke

koke commented Nov 6, 2013

Copy link
Copy Markdown
Member

Given that Podfile.lock is still in the repo, I think that's why pod install won't update pods to a newer version

@beaucollins

Copy link
Copy Markdown
Contributor

Right, you'll need to do pod update locally and commit the changed Podfile.lock if you want to change the versions used in the project.

@autobott

autobott commented Nov 7, 2013

Copy link
Copy Markdown

Build triggered.

@mikejohnstn

Copy link
Copy Markdown
Contributor

Note that after this is done, to get the first benefit above (a smaller initial download), you'll need to do some git wizardry to remove all the files from the commit history:
http://dalibornasevic.com/posts/2-permanently-remove-files-and-folders-from-a-git-repository

This is a risky operation (make sure you delete the right thing heh) but I've done it in the past for big binary files.

@koke

koke commented Nov 11, 2013

Copy link
Copy Markdown
Member

Please don't. Using that trick would rewrite all project history, invalidating every repo clone out there. Plus, it would rewrite history 👎

If you want a smaller download, use git clone --depth

@mikejohnstn

Copy link
Copy Markdown
Contributor

@koke The first benefit listed in the issue for this pull is "Smaller repo to download initially." I'm merely pointing out this won't be a benefit without some extra (risky) work.

@sendhil

sendhil commented Nov 11, 2013

Copy link
Copy Markdown
Contributor Author

I'm with @koke on this and am uncomfortable about the idea of rewriting the history, I know one of the goals was a smaller download size, but I think even with the current download size having no Pods directory is a win for us so we don't have to deal with people updating pods, pushing them to the repo, having to deal with possible merge conflicts, etc.

@koke

koke commented Nov 13, 2013

Copy link
Copy Markdown
Member

👍 then :)

@sendhil

sendhil commented Dec 9, 2013

Copy link
Copy Markdown
Contributor Author

Closing this and will redo it since this PR is super old and don't want to deal with an unnecessary merge nightmare.

@sendhil sendhil closed this Dec 9, 2013
@astralbodies
astralbodies deleted the issues/288 branch February 20, 2014 18:48
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.

Remove Pods from repository, only commit the podspecs

7 participants