search

grab / cocoapods-pod-merge

Cocoapods plugin to merge pods used by your Xcode project, reducing the number of dynamic frameworks your app has to load on startup
MIT License
375 stars 24 forks source link

Minimum deployment target #14

Open Vkt0r opened 4 years ago

Vkt0r commented 4 years ago

Hey there 👋. Thanks for the creation of this plugin it's really promising!

This is PR is focused on fixing an issue with the minimum deployment target in the *.podspec for the generated groups. In the plugin, this was hardcoded to s.ios.deployment_target = '8.0' causing that the inclusion of libraries with a higher deployment target would generate a compilation error. You can see the example library added in the PodMergeExample (Kingfisher).

To solve this I used the platform :ios, 'x.y' declared in the Podfile. This should reflect the minimum deployment target for the Pods project. It's worth to mention that the Podfile may have been parsed using the CocoaPods Core gem already available avoiding the manual parsing and regex, with something like this:

podfile = Pod::Podfile.from_file("Podfile")
sources = podfile.sources

podfile.root_target_definitions.each do |target|
    platforms.append(target.platform.to_s)
end

I leave as it was to avoid possibles issues in other parts of the plugin.

So this PR can be resumed in the following:

biocross commented 4 years ago

Thank you for your PR @Vkt0r!

I think reading the Podfile for the deployment target is a very nice! And thank you for the idea of reading the Podfile using Cocoapods Core, I should migrate some more code inside the plugin to use this and get rid my crazy regex 😂!

I've just left a few comments to better understand this PR :)

Vkt0r commented 4 years ago

@biocross I rebased this PR for the latest changes. Can you please continue? Thanks