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

Issue6765 proxy #6817

Merged
merged 3 commits into from Nov 2, 2016
Merged

Issue6765 proxy #6817

merged 3 commits into from Nov 2, 2016

Conversation

timothy-volvo
Copy link
Contributor

Thanks for contributing to fastlane! Before you submit your pull request, please make sure to check the following boxes:

  • Run bundle exec rspec from the subdirectory of each tool you modified. Alternatively, run rake test_all from the root directory.
  • Run bundle exec rubocop -a to ensure the code style is valid
  • Read the Contribution Guidelines
  • We currently don't accept new actions, please publish a plugin instead, more information in Plugins.md

Before submitting a pull request, we appreciate if you create an issue first to discuss the change 👍

PR for issue #6765

http_conn = Net::HTTP.new(uri.host, uri.port)
http_conn.use_ssl = true
result = http_conn.request_get(uri.path)
raise "#{result.message} (#{result.code})" unless result.kind_of? Net::HTTPSuccess
Copy link
Collaborator

@milch milch Nov 1, 2016

Choose a reason for hiding this comment

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

Please use UI.crash! instead of raise as described here: https://github.com/fastlane/fastlane/blob/master/fastlane/docs/UI.md

@@ -72,8 +72,12 @@ def download_android_tools
begin
UI.important("Downloading Crashlytics Support Library - this might take a minute...")

result = Net::HTTP.get(URI(url))
File.write(zip_path, result)
uri = URI(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to mention (with a link to the ruby issue) why it is done that way in a comment. Otherwise someone might come along later, wonder why it is done this way, and change it back.

@milch
Copy link
Collaborator

milch commented Nov 2, 2016

Beautiful 💯

@milch milch merged commit 3042878 into fastlane:master Nov 2, 2016
@fastlane fastlane locked and limited conversation to collaborators Feb 4, 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

3 participants