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

Let Carrierwave generate urls. Fixes #123 #151 #155

Merged
merged 2 commits into from
Feb 21, 2015
Merged

Conversation

p8
Copy link
Collaborator

@p8 p8 commented Sep 8, 2014

The url generation by CarrierwaveDirect is complex and has issues (#129, #151).
Carrierwave already generates urls for us.
By letting Carrierwave generate the urls we can remove code from CarrierwaveDirect.
Another benefit is default support for private storage on S3 (#123).

I have this branch currently working smoothly on production with private S3 storage.
DirectUrl::direct_fog_url(with_path: true) should probably be deprecated as it just calls url.

p8 added 2 commits September 8, 2014 17:08
…> true)

The current implmentation has issues while Carrierwave already has a
working solution.
This also make direct_fog_url work with private storage on S3
@p8 p8 changed the title Use the Carrierwave generated urls instead Let Carrierwave generate urls. Fixes #123 #151 Sep 10, 2014
@lsimoneau
Copy link
Contributor

+1 for this. Resolves a URL-encoding issue we were experiencing.

@mdh
Copy link

mdh commented Oct 14, 2014

+1

@ombr
Copy link

ombr commented Oct 31, 2014

+1 had issues with encoding and it solved it ! Thanks !

@flynfish
Copy link

flynfish commented Nov 6, 2014

+1

@vesan
Copy link

vesan commented Nov 7, 2014

@nddeluca Have you had a chance of looking at this?

@vinniefranco
Copy link
Contributor

👍

1 similar comment
@mhenrixon
Copy link

👍

@mhenrixon mhenrixon mentioned this pull request Dec 30, 2014
@marcusmalmberg
Copy link

👍 Fixed my encoding issues.

sunny added a commit to sunny/carrierwave_direct that referenced this pull request Feb 6, 2015
@dwilkie
Copy link
Owner

dwilkie commented Feb 13, 2015

Anyone want to help maintaining this gem? See #83

@p8
Copy link
Collaborator Author

p8 commented Feb 18, 2015

@dwilkie Sure, I'd like to help.

@dwilkie
Copy link
Owner

dwilkie commented Feb 20, 2015

@p8 Awesome thanks. You've got full access rights now

@dwilkie
Copy link
Owner

dwilkie commented Feb 20, 2015

@p8 PM me your email address so you can push to Rubygems

@p8
Copy link
Collaborator Author

p8 commented Feb 21, 2015

Thanks!

p8 added a commit that referenced this pull request Feb 21, 2015
Let Carrierwave generate urls. Fixes #123 #151
@p8 p8 merged commit baab52f into dwilkie:master Feb 21, 2015
@vesan
Copy link

vesan commented Feb 22, 2015

@p8, great work! 👍

I know I'm pushing it but cutting a release would be swell :-)

@p8
Copy link
Collaborator Author

p8 commented Feb 25, 2015

Done :)

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.

10 participants