-
Notifications
You must be signed in to change notification settings - Fork 986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check for newer versions of cocoapods to avoid locking adding platforms on non synced pods repo #719
Conversation
Codecov Report
@@ Coverage Diff @@
## master #719 +/- ##
==========================================
+ Coverage 74.24% 74.32% +0.08%
==========================================
Files 11 11
Lines 1844 1850 +6
==========================================
+ Hits 1369 1375 +6
Misses 475 475
Continue to review full report at Codecov.
|
const patch = parseInt(semver[2]); | ||
|
||
// starting with 1.8.0 cocoapods now use cdn and we dont need to sync first | ||
if ((major >= 1 && minor >= 8 && patch >= 0) || toolOptions.ignore) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
maybe it's better this way:
if (((major > 1) || (major == 1 && minor >= 8 && patch >= 0)) || toolOptions.ignore) {
do not you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's maybe also sufficient to only check for major and minor version.
All that is missing now is resolving the AppVeyor test. Since AppVeyor runs on Windows and there is no |
… as those use CDN.
Done! |
@renanccastro When giving the code a thorough review, I noticed that we already get the cocoapods version passed in as part of the result from the It would be great to have a unit test that checks the behavior for pod versions starting with |
Starting on 1.8.0, cocoapods now use CDN to avoid having to sync the master repo, so we don't need to check anymore if the repo is populated.
Even more, the "pod setup" command now doesn't do anything, and newer versions of cocoapods are actually incompatible with cordova-ios, as the repository filled check will kick in and make it impossible to use it.
Error example with meteor and cocoapods 1.8.4:
✗ CocoaPods: The CocoaPods repo has not been synced yet, this will take a long
time (approximately 500MB as of Sept 2016). Please run
pod setup
first tosync the repo.
This also solves issue #700
Platforms affected
Motivation and Context
Description
Testing
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)