Skip to content

Fix xcode version comparison #48

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

Merged
merged 1 commit into from
Aug 15, 2014
Merged

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Aug 14, 2014

Xcode version can be 3 numbers separated by dot or 2 numbers separated by dot. versionCompare function expects versions in same format. We cannot use semver module because it throws error for versions in "x.y" format.

@@ -35,6 +35,10 @@ class IOSProjectService implements IPlatformProjectService {
}

var xcodeBuildVersion = this.$childProcess.exec("xcodebuild -version | head -n 1 | sed -e 's/Xcode //'").wait();
if(xcodeBuildVersion.split(".").length === 3) {
xcodeBuildVersion = xcodeBuildVersion.slice(0, -2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is possible that xcode version goes above 10. I would feel much more comfortable if you combine the first two parts from the split string or using a regular expression like /(d+.d+)/ (to capture the first two number parts).

@Fatme Fatme closed this Aug 14, 2014
@Fatme Fatme reopened this Aug 15, 2014
@Fatme
Copy link
Contributor Author

Fatme commented Aug 15, 2014

@teobugslayer

@teobugslayer
Copy link
Contributor

👍

Fatme added a commit that referenced this pull request Aug 15, 2014
@Fatme Fatme merged commit b96c572 into master Aug 15, 2014
@Fatme Fatme deleted the fatme/fix-xcode-version-comparison branch August 15, 2014 11:14
rigor789 pushed a commit that referenced this pull request Feb 7, 2022
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.

2 participants