Subspecs support on the MergeFile #22
Conversation
|
@biocross Any change you can take a look into the 3 PRs solving issues in the project ?? |
|
@biocross Any chance you can take a look into the 3 PRs solving issues in the project ?? |
|
Hello @Vkt0r, really appreciate the PRs! I'll take a look this weekend and sometime next week. Thanks again! |
|
@biocross Great !! Thank you!!! I do have more PRs to fix more issues with the |
| @@ -188,7 +188,8 @@ def parse_mergefile | |||
| def merge(merged_framework_name, group_contents, podfile_info) | |||
| Pod::UI.puts "Preparing to Merge: #{merged_framework_name}" | |||
|
|
|||
| pods_to_merge = group_contents['titles'] | |||
| # Replace the Subspecs notation (e.g AppAuth/Core) for `AppAuth` only as the folder AppAuth/Core would not exist | |||
| pods_to_merge = group_contents['titles'].map { |pod_name| pod_name.sub /(\/[a-zA-Z]+)/,''}.uniq | |||
There was a problem hiding this comment.
Can elaborate exactly what this regex does? Will it remove any other symbols from a Pod name except /? Asking because I've come across pod names with symbols like _ , -
There was a problem hiding this comment.
Sure the purpose of the regex is to replace subspecs like AppAuth/Core for AppAuth as the folder AppAuth/Core would not exist. Do you have any names of pods with the _, - in mind we can test ? Maybe I missed those cases 😞.
There was a problem hiding this comment.
Sure, for example: https://cocoapods.org/pods/Google-Mobile-Ads-SDK
There was a problem hiding this comment.
That one doesn't have sub-specs so it doesn't affect anything related to the changes. Also that one is a vendor framework so it would fail. For example I added this youtube-ios-player-helper to the MergedSwiftPods and it works successfully.
Nevertheless I detected an error that could be worth to fix later and it's related to the dashes in the module map. If I add the youtube-ios-player-helper to the Subspecs group it would fail because the module.modulemap doesn't like dashes in the definition of modules
explicit module youtube-ios-player-helper {
header "YTPlayerView.h"
}And I think that's why most of the pods using dashes they also define module_name (I have a PR coming soon to fix an issue related to this with lottie-ios as pods with module_name defined should use the module_name instead of the name of the pod specification for imports)
|
is it on going? |
Hey 👋 ! This PR is focused on add support for Subspecs in the
MergeFile.The issue was the
Subspecswere treated as they were by the plugin. IfAppAuth/Corewas added to theMergeFilethe plugin was expecting a folder calledAppAuth/Corewould exist and this is not how this is handled. Two specific changes were made to support this:AppAuth/Corewill beAppAuthonly.AppAuth_Coreso it's necessary to keep control of the original list to replace it and add the corresponding Podspecs.This PR can be resumed in the following:
MergeFile.MergeFileto test the functionality,Closes #11