Feature/specs sources #19
Conversation
| # @!group Attributes | ||
|
|
||
| # @return [Array<Source>] all the specs source locations. |
There was a problem hiding this comment.
This method should only return a list of strings as the Podfile should't be concerned with the source class. The actual matching should be done by the Pod::Installer::Analyzer (or by a helper class under its namespace).
|
Looks good to me, other than those minor points that I mentioned. Once this is done I think that the support for CocoaPods/CocoaPods#982 in Core is complete. |
|
👍 |
|
@netbe any update on this? |
|
@irrationalfab yep I have to finish this up. But I guess i will finish first CocoaPods/CocoaPods#982 and get back on this as there might be other change. I think at some point we said we should specify names instead of url and then it's up to user to add the repo manually. or i can modify core now to support |
|
Thanks for the update I was just checking if the issue was still active. I don't recall the discussion about the URL. I think that we could start with just the names and add the cloningfunctionality later. On Fri, Aug 2, 2013 at 12:54 PM, François Benaiteau
|
|
@irrationalfab trying to get back to this old PR. Sorry for that, I mainly change the sources to be name of repos instead of urls |
| end | ||
|
|
||
| # @return [Array<Source>] all the sources. | ||
| # | ||
| def all | ||
| @sources ||= dirs.map { |repo| Source.new(repo) }.sort_by(&:name) |
There was a problem hiding this comment.
FYI I removed the sort_by in order to have the priority (natural order)
There was a problem hiding this comment.
I would be nice to have some specs to check that the it handles the given sources and that priority the order is respected. Please add a note about the list being ordered by importance.
|
Nice the Core patch is shaping up! |
|
Btw, we are aiming for a 100% Coverage in this repo. |
|
@irrationalfab yeah i ll try to add a test for the case where repo_dirs is an array, but in my opinion it could always be an array as SourceManager.agggregate does not cover all the cases. The one without source specify = sources = ["master"] Also would be nice to have the podifle.lock contain the source somehow, what do you think about that? |
I think that was my point as well, client should provide the paths of the sources as an array.
The Podfile should suffice imo, but I'm not sure. I would not worry about that atm. |
|
@irrationalfab Ok i think for the Core, there is no more stuff to do, maybe a more precise comment on source priorities, I basically took the same description as for Gemfile. But I was surprised to see the last added source was the highest priority. ie: Podfile: Priority order: source1 then source2 For the default source I will handle it in cocoapods gem. |
|
👍 Regarding the source priority in bundler I interpret it in the same way. I generally prefer to adopt their conventions to avoid gotchas but in this case the priority as proposed by you makes much more sense! I think that this can be merged when the patch in CocoaPods/CocoaPods is ready 🍻 |
Also updating previous work according to comments of pull request.
instead of default one
Order of repo sources does matter now. But in this test it does not. So for asserting we make sure we have the same order - alphabetic order
Removed private helper for master repo
Also updating previous work according to comments of pull request.
instead of default one
Removed private helper for master repo
| @@ -5,7 +5,7 @@ module Pod | |||
|
|
|||
| describe 'Set Information' do | |||
| before do | |||
| set = Source::Aggregate.new(fixture('spec-repos')).search_by_name('JSONKit').first | |||
| set = Source::Aggregate.new([fixture('spec-repos/master'), fixture('spec-repos/test_repo')]).search_by_name('JSONKit').first | |||
|
Closed in favour of #160 |
This pull request partially fixes issue CocoaPods/CocoaPods#982
It had the ability to specify other specs repositories sources with the following syntax: