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

Add skip_package_ipa option to Gym #8734

Merged
merged 1 commit into from Apr 21, 2017

Conversation

taquitos
Copy link
Collaborator

Sometimes you don’t want an ipa but rather, just the xcarchive.

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Description

Add skip_package_ipa option to Gym so after building the xcarchive we don't try to package up the ipa. Gym will return the path to the archive.

Motivation and Context

Some company build systems don't give access to certificates and they want to package up an ipa their way. This way you can output an xcarchive and hand it off to the internal build system.

Sometimes you don’t want an ipa but rather, just the xcarchive.
ohayon
ohayon previously requested changes Mar 31, 2017
Copy link
Contributor

@ohayon ohayon left a comment

Choose a reason for hiding this comment

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

Just one little thing about optionality. But otherwise this is 💯

FastlaneCore::ConfigItem.new(key: :skip_package_ipa,
env_name: "GYM_SKIP_PACKAGE_IPA",
description: "Should we skip packaging the ipa?",
is_string: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have this be optional: true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking about that. Instead of optional, I just put default_value: false since having both seems a bit redundant.
I figure it's never optional if there is always a default_value

Copy link
Member

Choose a reason for hiding this comment

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

I actually provided the opposite feedback, however both work: #8721

Copy link
Contributor

Choose a reason for hiding this comment

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

NVM! Realized that there is the default_value: false there! 👍

@mfurtak
Copy link
Contributor

mfurtak commented Mar 31, 2017

Are there tests we can add to ensure that this stays working in the future?

@KrauseFx
Copy link
Member

@mfurtak we currently don't have any tests for runner.rb 😢

@mfurtak
Copy link
Contributor

mfurtak commented Mar 31, 2017

@KrauseFx @taquitos Could we? 😄

@taquitos
Copy link
Collaborator Author

@mfurtak 🤔
I don't see why not. I'll work on that, maybe Monday 👍

Copy link
Member

@KrauseFx KrauseFx left a comment

Choose a reason for hiding this comment

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

The code is great, and it's something other people asked for too #8944

We can either add tests now, or create a task for this, your call ☺

@taquitos
Copy link
Collaborator Author

I think I'd like to create a task to do the testing. It seems rather complicated to get the runner test up and running, and this is a trivial update.

@taquitos taquitos merged commit 22612cd into fastlane:master Apr 21, 2017
@taquitos taquitos deleted the skip_packaging_ipa branch April 21, 2017 02:17
@stefanluptak
Copy link

Do I understand it correctly, that now I can do something like this?

gym(skip_packaging_ipa: true)
gym(archive_path: "<WHAT_TO_PUT_HERE?>", export_method: 'development')
gym(archive_path: "<WHAT_TO_PUT_HERE?>", export_method: 'app-store')

And what to use instead of "<WHAT_TO_PUT_HERE?>"? :-)

@fastlane-bot
Copy link

Hey @taquitos 👋

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 🚀

@fastlane-bot
Copy link

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

@stefanluptak
Copy link

I am sorry to be annoying, but do somebody know the answer to my question (few posts above) please? :-)

@KrauseFx
Copy link
Member

@EskiMag Please ask questions by submitting a new issue for this repo. Also, it seems like your question is not related to this PR.

@fastlane fastlane locked and limited conversation to collaborators Jul 23, 2017
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

7 participants