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

Fix irregular ios version by using getPlatformVersionsFromFileSystem #36

Closed

Conversation

knight9999
Copy link
Contributor

@knight9999 knight9999 commented Aug 8, 2017

This PR is like #35.

Instead of the webpack dependent method __non_webpack_require__ ,
This PR uses the getPlatVersionsFromFileSystem method which is same as the one defined in cordova-lib/platform_metadata.js.


PR #35 is the following.

iOSPlatform version should be the semver form, like "4.4.0".
However, If the user add platform ios by using the directory name or git:// case, like

cordova platform add /Home/user/cordova/ios/cordova-ios4.4.0

The iOSPlatform version is Home/user/cordova/ios/cordova-ios4.4.0 not 4.4.0.
(See
https://github.com/apache/cordova-lib/blob/master/src/cordova/platform_metadata.js
and its comment above the getVersions method)

In such case, this plugin hook script does not work.

This pull request, the correct iOSPlatform version is obtained by accessing platform/ios/cordova/version file.

@akofman
Copy link
Owner

akofman commented Aug 8, 2017

Thanks ! Before merging it, I'm just going to check with the Cordova team if it is possible to expose this function. I don't understand why it's not the case and this will prevent us to copy/paste their sources.

@knight9999
Copy link
Contributor Author

I see. Exposing the function getPlatVersionsFromFileSystem is usable.

@Mischa1610
Copy link

For me/us would this Pull Request be also pretty important because we need to use the master branch for cordova-ios because there are fixes regarding XCode 9 and they are not sure when a new version of cordova-ios will be released.

@jcesarmobile
Copy link

This probably fixes #43

@akofman
Copy link
Owner

akofman commented Dec 22, 2017

You're right ! I'm fixing the conflicts and merge this one.

@akofman
Copy link
Owner

akofman commented Dec 22, 2017

Squashed and merged manually. thx @knight9999 and sorry for the late.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants