-
Notifications
You must be signed in to change notification settings - Fork 147
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
feat(shorebird_cli): add xcodeVersion method to xcodebuild wrapper #1360
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1360 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 152 152
Lines 4434 4446 +12
=========================================
+ Hits 4434 4446 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
); | ||
} | ||
|
||
// [Version.parse] requires a patch number. If Xcode does not report a patch |
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.
I wonder if this will work for all XCode versions and/or whether it would be easier and less error-prone to try to parse the version as a double and see if it’s greater than 15 since clearly XCode doesn’t follow semver I’m not sure this would be the only issue we encounter (for example if there’s a beta version of XCode do we handle that correctly with this logic). 🤷♂️
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.
I guess we should probably be consistent with the flutter_tool in this case
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.
Good Q about the beta, let me download one and give that a shot
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.
Interestingly, Xcode betas don't report that they're beta in xcodebuild -version
:
⑆ xcrun xcodebuild -version
Xcode 15.1
Build version 15C5028h
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.
To resolve this, I don't think parsing it as a double would work, as there is occasionally a patch version.
Description
Adds the ability to get the currently installed Xcode version using
xcodebuild -version
.Type of Change