DateTime#to_time should always return a Time

It's worked this way for all 2.x releases; we'd really need a good reason to change this behavior now.

Do you have a specific use case where you run into an issue with this behavior?

Well, I have: I our app we need to turn all DateTime objects to Time objects and right now it's not possible. We've just created a helper, so it doesn't really matter to us, but it's kinda weird DateTime#to_time doesn't always return a Time (even though it's documentated).

Sometime in the future I think this needs to be merged; there's no point of not doing it when it's perfectly possible.

I've also included the patch as an attachment, in case I'll delete my Rails-repo :-)

Your implementation does a time zone conversion, which is adding new behavior to this method. The resulting object is equivalent in a #to_i sense (i.e., seconds since epoch), but it's not equivalent in a time values sense -- here's an example to help explain:

# DateTime#to_time behavior with above patch:
>> DateTime.now.to_s(:long)
=> "March 14, 2009 11:04"
>> DateTime.now.to_time.to_s(:long)
=> "March 14, 2009 16:04"

The current behavior of DateTime#to_time only returns a Time object if it cleanly maps to an equivalent Time object in both a #to_i sense and a time values sense. Given that ActiveSupport ducktypes DateTime objects to respond like Time objects for most of the common methods we need use, you can still do most everything you want with the returned DateTime object, so this seems like a reasonable fallback.

Interestingly, Ruby 1.9's DateTime#to_time always converts to the system local zone, similar to your patch, just with a different zone:

irb(main):006:0> DateTime.now.to_time
=> 2009-03-14 11:20:01 -0500
irb(main):007:0> DateTime.now.new_offset(0).to_time
=> 2009-03-14 11:20:08 -0500

A reasonable argument could be made that we should be following this behavior instead of what's proposed here.

I do think the current ActiveSupport behavior works well for us, given the additional ducktyping support for DateTimes. Given this, and the fact that this patch would potentially cause unexpected behavior, I'm -1 on this.