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 query parameter support for fog-google #2332

Merged

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Aug 16, 2018

@stanhu stanhu changed the title Add query parameter support for fog-google WIP: Add query parameter support for fog-google Aug 16, 2018
@icco
Copy link
Contributor

icco commented Aug 19, 2018

FYI, you'll probably need to update https://github.com/carrierwaveuploader/carrierwave/blob/master/carrierwave.gemspec#L37 to make this work.

@stanhu stanhu changed the title WIP: Add query parameter support for fog-google Add query parameter support for fog-google Aug 19, 2018
@stanhu
Copy link
Contributor Author

stanhu commented Aug 20, 2018

@icco Oops, the build is failing here due to https://github.com/fog/fog-google/pull/412/files.

@stanhu
Copy link
Contributor Author

stanhu commented Aug 21, 2018

Build failures are unrelated and related to #2333.

@stanhu
Copy link
Contributor Author

stanhu commented Sep 13, 2018

@mshibuya Could you take a look here? We're shipping this as a monkey patch in GitLab at the moment.

@stanhu stanhu force-pushed the sh-add-query-params-google branch 2 times, most recently from d7c8c56 to 31e2b92 Compare December 16, 2018 23:05
@mshibuya
Copy link
Member

May I ask you one more favor, could you make this spec cover fog-google?

it "should handle query params" do
if @provider == 'AWS' && !Fog.mocking?
headers = Excon.get(@fog_file.url(:query => {"response-content-disposition" => "attachment"})).headers
expect(headers["Content-Disposition"]).to eq("attachment")
end
end

@stanhu
Copy link
Contributor Author

stanhu commented Dec 23, 2018

May I ask you one more favor, could you make this spec cover fog-google?

Sure, good idea. Done.

@mshibuya mshibuya merged commit 0b2e085 into carrierwaveuploader:master Dec 24, 2018
@mshibuya
Copy link
Member

Nice work, thanks!

@stanhu
Copy link
Contributor Author

stanhu commented Dec 24, 2018

@mshibuya Would you mind tagging a release now? Thanks!

@mshibuya
Copy link
Member

Sure, published 1.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants