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

Fix broken calls to get_object_http_url #412

Merged
merged 1 commit into from
Aug 21, 2018

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Aug 20, 2018

Also makes the option parameter in get_object_https_url optional.

@@ -6,12 +6,11 @@ def get_object_http_url(bucket_name, object_name, expires, options = {})
raise ArgumentError.new("bucket_name is required") unless bucket_name
raise ArgumentError.new("object_name is required") unless object_name

https_url(options.merge(:headers => {},
http_url(options.merge(:headers => {},

Choose a reason for hiding this comment

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

Layout/SpaceAroundOperators: Operator => should be surrounded by a single space.

Also makes the option parameter in get_object_https_url optional.
@stanhu stanhu force-pushed the sh-fix-broken-get-http-url branch from f0975ed to 154bf89 Compare August 20, 2018 04:10
Copy link
Member

@icco icco left a comment

Choose a reason for hiding this comment

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

Makes sense to me. cc @Temikus wanna merge since you reviewed the first change to this?

@icco
Copy link
Member

icco commented Aug 20, 2018

Relates to carrierwaveuploader/carrierwave#2332

@Temikus
Copy link
Member

Temikus commented Aug 21, 2018

That is my mistake, should've noticed it during #409 - I'll pull this in when tests pass and roll a new release out.

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #412 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #412   +/-   ##
=======================================
  Coverage   84.11%   84.11%           
=======================================
  Files         339      339           
  Lines        5771     5771           
=======================================
  Hits         4854     4854           
  Misses        917      917

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa2e560...154bf89. Read the comment docs.

@Temikus Temikus merged commit 9283fe6 into fog:master Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants