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

Fixed Get Apple OSX Version method #376

Merged
merged 1 commit into from
Jul 31, 2018

Conversation

erisu
Copy link
Member

@erisu erisu commented Jun 23, 2018

Platforms affected

iOS

What does this PR do?

While working on an upcoming PR (#377 ) for improving the test coverage, I noticed that the get_apple_osx_version method has been returning undefined.

After further investigation, the RegEx pattern has been looking for OSX SDK version OS X no longer exists since Xcode 8+

Example print outs:
Xcode 7.3: https://travis-ci.org/erisu/macos-test/jobs/395785417
Xcode 8: https://travis-ci.org/erisu/macos-test/jobs/395785418
For other versions: https://travis-ci.org/erisu/macos-test/builds/395785415

From the above print out, I determined that the RegEx pattern should be updated to macOS

What testing has been done on this change?

Note: Oddly Node 4 appears to be failing but I have submitted PR #375 to drop Node 4 support.

Checklist

  • ~Reported an issue in the JIRA database~~
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

Modified RegEx pattern to look for new SDK pattern 'macOS'
@erisu erisu mentioned this pull request Jun 23, 2018
1 task
@brodycj brodycj self-requested a review July 31, 2018 13:09
Copy link
Contributor

@brodycj brodycj left a comment

Choose a reason for hiding this comment

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

<3

@brodycj brodycj merged commit f967fc4 into apache:master Jul 31, 2018
@brodycj
Copy link
Contributor

brodycj commented Jul 31, 2018

I discovered a few TODO items after merging:

P.S. I took the liberty to edit the Checklist section.

brodycj pushed a commit to brodycj/cordova-ios that referenced this pull request Jul 31, 2018
(was part of apacheGH-377)

covers update from apacheGH-376

Co-authored-by: エリス <[email protected]>
Co-authored-by: Christopher J. Brody <[email protected]>
brodycj pushed a commit to brodycj/cordova-ios that referenced this pull request Jul 31, 2018
(was part of apacheGH-377)

covers update from apacheGH-376

Co-authored-by: エリス <[email protected]>
Co-authored-by: Christopher J. Brody <[email protected]>
brodycj pushed a commit to brodycj/cordova-ios that referenced this pull request Jul 31, 2018
(was part of apacheGH-377)

covers update from apacheGH-376

Co-authored-by: エリス <[email protected]>
Co-authored-by: Christopher J. Brody <[email protected]>
brodycj pushed a commit that referenced this pull request Jul 31, 2018
(was part of GH-377)

covers update from GH-376

Co-authored-by: エリス <[email protected]>
Co-authored-by: Christopher J. Brody <[email protected]>
erisu added a commit to erisu/cordova-ios that referenced this pull request Jan 16, 2019
(was part of apacheGH-377)

covers update from apacheGH-376

Co-authored-by: エリス <[email protected]>
Co-authored-by: Christopher J. Brody <[email protected]>
@erisu erisu deleted the fix-get-apple-osx-version branch April 4, 2019 06:15
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