cc @josevalim @jeremy

1. If we're exposing this for reuse, we need docs! How should a library author use this?
2. Feels like libraries using separate deprecators should be working with instances of a class rather than extending with a module that injects a bunch of behavior, including its internal state. Could make ActiveSupport::Deprecation a class (without breaking API) and delegate the existing toplevel methods to a singleton ActiveSupport::Deprecation.instance
3. The #deprecator instance method may be unnecessary. Looks like the only reason we need it is so the deprecated method wrapper can refer to it. It'd be cleaner to pass a deprecator instance and reference it directly from the method wrapper (using define_method instead of class_eval)

@jeremy done :)

@LTe Very nice! Looking good.

@carlosantoniodasilva @jeremy thanks for feedback!

What do you think about it:

module ActiveSupport
  class Deprecation
    def initialize(horizon = '3.2', gem_name = 'rails')
      self.deprecation_horizon = horizon
      self.gem_name = gem_name
    end
  end
end

For custom gems authors should create new instances of ActiveSupport::Deprecation to generate proper warning messages. If they use ActiveSupport::Deprecation.instance the message is going to say that something will be removed from rails which is in many cases not what the authors of gems would like to announce to the users.

These:

deprecator = ActiveSupport::Deprecation.new
deprecator.deprecation_horizon = "2.0.0"

ActiveSupport::Deprecation.new.tap{|d| d.deprecation_horizon = "2.0.0" }

seem to be unnecessary verbose to me.

ActiveSupport::Deprecation.new("2.0.0", "formtastic")

looks much better imho.

Science Fiction:

module ActiveSupport
  class Deprecation
    def initialize(horizon = default_horizon, gem_name = 'rails')
      self.deprecation_horizon = horizon
      self.gem_name = gem_name
    end

    def default_horizon
      "#{ActiveSupport::VERSION::MAJOR}.#{ActiveSupport::VERSION::MINOR + 1}"
    end
  end
end

because sometimes it should be major + 1. Or maybe it should be always major +1 and never minor + 1 ?

Now you can create instance of deprecator and create new object via new.
I like @paneq approach -- now this is simple to reuse.

About science fiction I think better solution is pass directly version to initialize method.

"About science fiction I think better solution is pass directly version to initialize method." - probably. Except for the fact that the core team needs to change this one line every time there is new rails version. I wanted to spare them this trouble in this science fiction solution :)

According to http://semver.org/ this always should be major +1

@jeremy what do you think? We can merge current implementation?

@jeremy updated and rebased

@LTe some comments on the documentation :)

It covers how to give a custom deprecator object, but doesn't show how you can use ActiveSupport::Deprecation.new(version, libname) yourself.

When I read through the duck type for a deprecator, having to implement two methods seems wrong, too. We call deprecator.deprecated_method_warning then pass its return value to #warn -- we could just call one method and let the deprecator handle that internally.

@jeremy - I think that's because #warn has the logic for outputing to logger, stderr, output or whatever is configured in current development environment and #deprecated_method_warning is just responsible for generting the warning. One might want to generate different warnings for his own gem but keep the logic coherent with Rails as to where they are displayed. What do you think ?

@paneq definitely -- and a higher-level method like deprecation_warning(deprecated_method_name, message, caller_backtrace) could wrap that up. Then custom classes can implement that one method whereas subclasses of ActiveSupport::Deprecation can override just deprecated_method_warning if they like.

Updated

@jeremy what do you think about merge?

@LTe it needs rebased at least, it can't be merged cleanly anymore.

@steveklabnik rebased ;-)

Thanks. Let's see what @jeremy says.

What do you think @jeremy ?

👍

@LTe hey, could you please add a changelog entry for this, and check the minor comments I made, so that we can get this in master? Thanks!

@carlosantoniodasilva updated, rebased.

@LTe, great thanks. I'll have to ask you one more thing though, to review your commit message and improve the changelog a bit with an example of how to use this new feature. This will help others to understand the reasoning and how to use the feature at the same time when reading the commits or the changelog.

Here's an explanation about how to go with the commit message, and here's another about changelogs.

I'll merge it afterwards. Thanks!

@carlosantoniodasilva updated

@LTe great, thank you!