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

Unguard xcode version check in FastlaneCore::ItunesTransporter #11184

Merged
merged 1 commit into from
Dec 12, 2017

Conversation

janpio
Copy link
Member

@janpio janpio commented Dec 12, 2017

Quite some time ago in #4631 ("Only query for xcode version on macs") a Helper.is_mac? && was added as "guard" before Helper.xcode_version.start_with?('6.'), so only if run on mac, the xcode version is actually checked:

use_shell_script ||= Helper.is_mac? && Helper.xcode_version.start_with?('6.')

Unfortunately I could not find out why this was done. I assume it was done to make this code not crash on system that do not have Xcode installed - which may have raised an exception if you went to check for the version.

Unfortunately it has the side effect of also making tests, that mock this xcode version check to the required 6.x version so the ShellScriptTransporterExecutor is used, not pass on non-Mac platforms.

Fortunately Helper.xcode_version got its own guard a few months ago in PR #9998:

return nil unless self.is_mac?

With this new check, the additional check in ItunesTransporter is not necessary any more (as the version will be returned as nil on non-Mac platforms instead of raising an exception) and can be removed - which this PR does.

@taquitos
Copy link
Collaborator

Huh, nice find, and extremely thorough analysis!

Copy link
Collaborator

@taquitos taquitos left a comment

Choose a reason for hiding this comment

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

🎈🐐

@taquitos taquitos merged commit 4d129bd into master Dec 12, 2017
@janpio
Copy link
Member Author

janpio commented Dec 12, 2017

I want to note that I first removed that is_mac check, and only later figured out why it worked. 🤣
While posting my PR I then even found the PR responsible for it 😆

@janpio janpio deleted the janpio-unguard_xcode_version_check branch December 12, 2017 23:06
@fastlane-bot
Copy link

Hey @janpio 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

@mpirri mpirri mentioned this pull request Dec 13, 2017
mpirri added a commit that referenced this pull request Dec 13, 2017
@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.69.1 🚀

@fastlane fastlane locked and limited conversation to collaborators Feb 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants