Didn't the feature mention warning when Fixnumis directly referenced? Should we check for the ruby version explicitly instead?

It's undecided; there's no warning currently. It may be too noisy to tolerate for widespread use. Prefer feature detection to implementation version checks.

Fair enough. If nothing else we could move the Integer.prepend case outside of the conditional. Perhaps something like this:

Integer.prepend(ActiveSupport::NumericWithFormat)

if defined?(Bignum) && Bignum.ancestors.exclude?(ActiveSupport::NumericWithFormat)
  Bignum.prepend(ActiveSupport::NumericWithFormat)
end

Thanks for the suggestions. That prepends Fixnum's ancestor, not Fixnum, so no go.

To avoid referencing Fixnum entirely, the simple route is checking 1.class == Integer. I didn't litter the code with this (there are half a dozen other cases) because it's speculating how—and whether—Fixnum references will spam warnings. Poor trade for unknown benefit. There's little drawback to waiting to see what drops on Ruby trunk.

👍

On Wed, May 18, 2016, 6:30 PM Jeremy Daer notifications@github.com wrote:

In activesupport/lib/active_support/core_ext/numeric/conversions.rb
#25056 (comment):

@@ -134,6 +134,12 @@ def to_formatted_s(*args)
deprecate to_formatted_s: :to_s
end

-[Fixnum, Bignum, Float, BigDecimal].each do |klass|

• klass.prepend(ActiveSupport::NumericWithFormat)
  +# Ruby 2.4+ unifies Fixnum & Bignum into Integer.
  +if Integer == Fixnum

Thanks for the suggestions. That prepends Fixnum's ancestor, not Fixnum,
so no go.

To avoid referencing Fixnum entirely, the simple route is checking 1.class
== Integer. I didn't litter the code with this (there are half a dozen
other cases) because it's speculating how—and whether—Fixnum references
will spam warnings. Poor trade for unknown benefit. There's little drawback
to waiting to see what drops on Ruby trunk.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/rails/rails/pull/25056/files/01d1a9f97fdda379ed47a737d2416b2139209cb7#r63794236