search

adzap / validates_timeliness

Date and time validation plugin for ActiveModel and Rails. Supports multiple ORMs and allows custom date/time formats.
MIT License
1.59k stars 227 forks source link

rails 4.1 deprecates Validator#setup #114

Closed razum2um closed 10 years ago

razum2um commented 10 years ago

looks some kind of ugly, but i'd rather not to bump version because of this change

# DEPRECATION WARNING: The `Validator#setup` instance method is deprecated
and will be removed on Rails 4.2.
jaredbeck commented 10 years ago

looks some kind of ugly

Yeah, the alias_method seems like a bit of a hack.

However, I've tried out this patch in my own project, and can confirm that the deprecation warnings are gone, and all my tests still pass. Hope that helps.

gem 'validates_timeliness',
  github: 'razum2um/validates_timeliness',
  ref: 'b195081f6aeead619430ad38b0f0dfe4d4981252'

cc: @adzap

jaredbeck commented 10 years ago

Here's another solution from @yabawock.

razum2um commented 10 years ago

yes, it's better to rely on respond_to? :deprecated_setup than on a version please, prefer his solution

jaredbeck commented 10 years ago

Hmm I'm not sure you should close this, unless there's another PR I'm not seeing.

On Monday, April 7, 2014, Vlad Bokov notifications@github.com wrote:

Closed #114 https://github.com/adzap/validates_timeliness/pull/114.

Reply to this email directly or view it on GitHubhttps://github.com/adzap/validates_timeliness/pull/114 .

Jared Beck * (607) 216-5373 * jared@jaredbeck.com

johncarney commented 10 years ago

Actually, respond_to? :deprecated_setup is no good. First because deprecated_setup is a private instance method, so ActiveModel::Validator.respond_to? :deprecated_setup will always return false.

Replacing it with private_instance_methods.include? :deprecated_setup works, but then you have a small problem of what happens when deprecated_setup is removed altogether in Rails 4.2. You'll end up with a redundant method definition (or alias). This probably isn't too bad and you can mitigate the risk of future conflict by adding method_defined? :setup to the conditional.

I've sent a pull request with this solution: #117

felixbuenemann commented 10 years ago

I've yet another approach to this using version detection: felixbuenemann/validates_timeliness@b04193d7ea7445a4cb754cab52ca60f39e51da65

johncarney commented 10 years ago

I prefer a method based on actual behaviour rather than version numbers, but I do like the way you've put it in a constant.

felixbuenemann commented 10 years ago

I think your version is fine for this case.

jaredbeck commented 10 years ago

:+1: @felixbuenemann I like the constant too. Let's move this conversation over to #117 since this PR is closed.