uploadcare / uploadcare-rails

Rails API client that handles uploads and further operations with files by wrapping Uploadcare Upload and REST APIs.
https://uploadcare.com
MIT License
58 stars 70 forks source link

Change callback generation to use symbols instead of strings #51

Closed brandoncordell closed 7 years ago

brandoncordell commented 7 years ago

In Rails 5.1.0.beta1 passing a string to create a callback was deprecated. It's moved past deprecation and not throws an ArgumentError.

I've changed the callback generation to use symbols instead of strings.

See: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/callbacks.rb#L283

dmitry-mukhin commented 7 years ago

@vizvamitra can you look at this please?

brandoncordell commented 7 years ago

I'm not sure if uploadcare has a style guide, it may be cleaner to before_save "check_#{attribute}_for_uuid".to_sym

vizvamitra commented 7 years ago

@dmitry-mukhin we can merge it because this change won't break anything. Also we should bump patch version.

Also we should think about rails 5.1 support since it is already in rc1. I've just tried to run some tests against rails 5.1 and 20% of them are failing, but this is probably because of the tests themself.

dmitry-mukhin commented 7 years ago

@vizvamitra what about earlier rails?

brandoncordell commented 7 years ago

Registering a callback using a symbol is valid back to Rails 3.2 (and probably before that).

http://guides.rubyonrails.org/v3.2/active_record_validations_callbacks.html#callback-registration

brandoncordell commented 7 years ago

@vizvamitra There's also an issue with the generator with Rails 5.1.0.rc1. I'll create an issue and a pull request.

Pull request here: #54

vizvamitra commented 7 years ago

@dmitry-mukhin registering callbacks using symbols is supported since rails 2.3 (or maybe even earlier) so there shouldn't be any problems.

@brandoncordell change in schema.rb seems accidental and not related to the issue. Can you please remove it from the commit?

vizvamitra commented 7 years ago

Closing since fixed in #55