Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow non-ASCII filename by default. Rework of #1148 #1772

Merged

Conversation

shuhei
Copy link
Contributor

@shuhei shuhei commented Nov 16, 2015

BREAKING CHANGE: Allowing non-ASCII filename by default.

This is a rework of #1148 by @yalab.

@thomasfedb
Copy link
Contributor

LGTM ✨

@bensie Is it possible to either get master passing on Travis, or determine if CI failures in this build are related to the changes in this PR.

@bensie
Copy link
Member

bensie commented Nov 18, 2015

@thomasfedb I think there was a problem with Travis when this ran initially, re-running seems to be working fine. There is an issue with the rails-master Gemfile (Rails 5), but this PR isn't affecting that.

This looks great, thanks @shuhei!

bensie added a commit that referenced this pull request Nov 18, 2015
Allow non-ASCII filename by default. Rework of #1148
@bensie bensie merged commit 5b061c6 into carrierwaveuploader:master Nov 18, 2015
@shuhei
Copy link
Contributor Author

shuhei commented Nov 19, 2015

@bensie @thomasfedb Thanks for reviewing and merging!

@shuhei
Copy link
Contributor Author

shuhei commented Nov 19, 2015

@bensie Is there any change that this is gonna be released in the near future?

@thomasfedb
Copy link
Contributor

@shuhei This will be included in the 1.0.0 release, which there isn't a defined timeline for quite yet.

@shuhei
Copy link
Contributor Author

shuhei commented Nov 20, 2015

@thomasfedb I understand that this should be in a major upgrade because it breaks the current default. Thanks for all your effort for keeping this great library working!

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

Successfully merging this pull request may close these issues.

3 participants