Skip to content

Add CDN repo Source to allow retrieving specs from a web URL#469

Merged
dnkoutso merged 1 commit intoCocoaPods:masterfrom
igor-makarov:master
Dec 15, 2018
Merged

Add CDN repo Source to allow retrieving specs from a web URL#469
dnkoutso merged 1 commit intoCocoaPods:masterfrom
igor-makarov:master

Conversation

@igor-makarov
Copy link
Contributor

This PR complements #8280 in the main repo.

See all the details there.

@igor-makarov
Copy link
Contributor Author

I've made (hopefully) all the requested changes.

@igor-makarov
Copy link
Contributor Author

I've added a threadpool to handle all the concurrency, using ruby-concurrent.

In addition, I've moved all the optimizations that were previously in the analyzer, into the CDNSource file.

Copy link
Member

@amorde amorde left a comment

Choose a reason for hiding this comment

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

This is awesome! Would love to see tests for this before merging

@igor-makarov igor-makarov force-pushed the master branch 2 times, most recently from 1200768 to f6b0f23 Compare November 19, 2018 21:43
@igor-makarov
Copy link
Contributor Author

So I've picked all the nits, and made most of the implementation-related methods private.

I've begun adding unit tests. I'm using webrick for mocking the CDN server. I had to put its hookup in spec_helper to ensure no tests fail. I think it works better than VCR, because what I need is a read-only, static-hosted HTTP server that supports ETag out of the box.

@igor-makarov
Copy link
Contributor Author

I've added the tests for CDNSource. It's now 93% covered, according to what I can see.

@igor-makarov
Copy link
Contributor Author

I've pruned the new mock_cdn_repo_remote fixture to have significantly less files. Hope that's better.

Copy link
Member

@amorde amorde left a comment

Choose a reason for hiding this comment

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

Haven't gotten the chance to try it out yet but the diff looks good to me

@igor-makarov
Copy link
Contributor Author

I've made the changes requested by @segiddins.

@dnkoutso dnkoutso merged commit 87c71c4 into CocoaPods:master Dec 15, 2018
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.

4 participants