Skip to content
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

Update to Cordova 6.4.0 #8239

Merged
merged 4 commits into from Jan 25, 2017
Merged

Update to Cordova 6.4.0 #8239

merged 4 commits into from Jan 25, 2017

Conversation

@wojtkowiak
Copy link
Contributor

@wojtkowiak wojtkowiak commented Jan 11, 2017

This is a Cordova update made according to #8155
Testing it right now.

@wojtkowiak wojtkowiak changed the title [WIP] Cordova 6.4.0 Update to Cordova 6.4.0 Jan 13, 2017
@wojtkowiak
Copy link
Contributor Author

@wojtkowiak wojtkowiak commented Jan 16, 2017

I have finished testing

@abernix abernix self-assigned this Jan 16, 2017
@benjamn benjamn requested a review from abernix Jan 18, 2017
@abernix
Copy link
Member

@abernix abernix commented Jan 20, 2017

Apologies for the delay on reviewing this. This took me quite some time to evaluate (due to environmental delays, mostly: VirtualBox, Windows, Android SDK, etc.) but I'm fairly confident that these changes are all correct. I tested on Mac (Android and iOS) and Windows (Android only, as no iOS). Some points worth discussing before introducing this to (hopefully?) Meteor 1.4.3:

  • This upgrade requires installation of Android SDK 25, something which I don't think the last Android changes (6.2.0, for example) required (23 has been the "norm" for a while, I think). As seen in the changes for this PR, this was Meteor's first entry into the 6.x branch of cordova-lib. Cordova doesn't really make it clear on this page what is the correct API platform for the 6.x branch, though I believe after reading their blog that "API 25" is the correct move. This will need to be updated in the Android Prereqs of the Meteor Guide.
  • For whatever reason, in order to build for iOS, I needed to gem install cocoapods. I'm not sure if this is a new requirement or not, but it's not mentioned in the iOS Prereqs in the Meteor Guide and I don't recall needing this in the past (though it's been a while since I've built an iOS Cordova app and I'm on a new machine).
  • I'm fairly sure (though not certain) this will (intentionally) block hot-code push functionality (due to changing native code, etc.). I'm fairly sure that would have been the case during the upgrade to 6.2.0 as well (which landed in 1.4.2), but I guess it's worth mentioning.
@wojtkowiak
Copy link
Contributor Author

@wojtkowiak wojtkowiak commented Jan 23, 2017

Thanks for the review. I have conducted tests on Windows 10, Xubuntu 16.04 and newest MacOS.

Yeah, cordova page seems to be outdated. The SDK needed is indeed 25 -> apache/cordova-android@35b0ba6#diff-6899c18ece5b4dbbb91204b8356126d1 I will make PR to the guide.

gem install cocoapods not sure about this one. I do not have an access to OSX right now, but I will try to verify it this week.

About HCP - yes, it changes the compatibility version for sure because the plugin versions used by the packages like webapp, statusbar etc have been changed.

Copy link
Member

@abernix abernix left a comment

Ok, I'm in favor of this landing in 1.4.3. The remaining action items are doc related. We'll need to update History.md with a blurb similar to the 1.4.2 upgrade to 6.2.0.

Please do open that PR on the Guide, @wojtkowiak and link it to this issue, if you will. 😄

@abernix abernix added this to the Release 1.4.3 milestone Jan 23, 2017
@benjamn benjamn merged commit 355cde3 into meteor:devel Jan 25, 2017
2 checks passed
2 checks passed
CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GeorgeCalvert
Copy link

@GeorgeCalvert GeorgeCalvert commented Feb 28, 2017

FYI, phonegap-plugin-push v1.9.0 uses CocoaPods. See this.

@abernix
Copy link
Member

@abernix abernix commented Feb 28, 2017

@GeorgeCalvert What does that have to do with this Pull Request?

@GeorgeCalvert
Copy link

@GeorgeCalvert GeorgeCalvert commented Feb 28, 2017

@abernix, your review noted the unexpected dependency on CocoaPods. @wojtkowiak intended to verify but I didn't find any further comment. I just wanted to close the loop for others as our team too found the dependency unexpected. Maybe this warrants a note in history.md, but since that references this PR, a quick comment here seemed sufficient.

@abernix
Copy link
Member

@abernix abernix commented Feb 28, 2017

@GeorgeCalvert Ah, right! Did you also have to take action (e.g. gem install cocoapods)?

@GeorgeCalvert
Copy link

@GeorgeCalvert GeorgeCalvert commented Feb 28, 2017

Yep :-) Annoying move by Cordova IMHO.

skirunman added a commit to skirunman/guide that referenced this pull request May 11, 2017
More cleanup, clarifications, and fixes for mobile documentation. Add mention cocoapods dependency issue on iOS as per meteor/meteor#8239
lorensr added a commit to meteor/guide that referenced this pull request Jun 7, 2017
* Clarifications to mobile install and build process

More cleanup, clarifications, and fixes for mobile documentation. Add mention cocoapods dependency issue on iOS as per meteor/meteor#8239

* Fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants