Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

ActiveSupport 3.2.6 modular extensions missing require #6896

Closed
iamvery opened this Issue · 7 comments

4 participants

@iamvery

See https://gist.github.com/3012323

With ActiveSupport 3.2.3 I was able to require the active_support/core_ext/numeric/time by itself to get access to things like 4.minutes.ago, but doing the same in 3.2.6 throws an error in the ago method presumable because active_support/core_ext/time/zones is never required.

I tried for a bit to locate the appropriate location for the statement, but I'm not very familiar with the Rails source. I figured its an easy fix for someone who is...

@steveklabnik
Collaborator

In general, requires map directly to files. So if active_support/core_ext/time/zones is not required, you can find that file in activesupport/lib/active_support/core_ext/time/zones.rb

Same thing with activesupport/lib/active_support/core_ext/numeric/time.rb

@iamvery

I just noticed that even requiring the entire active_support/core_ext/numeric in 3.2.6 still causes the problem described. It looks like the only places that active_support/core_ext/time/zones is required is in the Date and DateTime extensions. Maybe it should probably be around in active_support/core_ext/time/calculations.rb?

I could be way off base, but I thought the whole design of ActiveSupport lent itself to being required in very small pieces to grab only what you need.

If you agree, I can put a pull request together for you if its worth it.

Thanks for you help :)

@steveklabnik
Collaborator

I could be way off base, but I thought the whole design of ActiveSupport lent itself to being required in very small pieces to grab only what you need.

This is true, but it can get complicated. See a similar discussion over in #6884.

That said, I'm not sure if this is a bug or not, exactly. @fxn?

@iamvery

I hear ya man. Thanks again for the speedy feedback!

@fxn fxn was assigned
@pferdefleisch

I think this is related.
In the docs it says that the rule of thumb to require classes is require 'active_support/core_ext/some_class'
This works for everything except for date, datetime and time.
Is there a reason for this?
I will try my best to add that pull request to this thread.

@pferdefleisch

As I was working on the pull request - I noticed active_support/time
My pull request is ready. Is it redundant?
Thanks!

@pferdefleisch pferdefleisch referenced this issue from a commit
@pferdefleisch pferdefleisch Added time related req files to AS core_ext #6896
This way you can `require 'active_record/core_ext/time'` for example
I see these libs are available through `active_record/time` but not
individually
4940dc2
@fxn
Owner

I don't think so, AS core extensions follow that pattern of having a single entry point to group extensions to a given class, I believe we have better consistency with the files you sent.

@fxn fxn closed this issue from a commit
@fxn fxn adds a missing require [fixes #6896]
This file uses Time.zone, which is defined in
active_support/core_ext/time/zones.rb.
aa6f512
@fxn fxn closed this in aa6f512
@fxn fxn referenced this issue from a commit
@fxn fxn adds a missing require [fixes #6896]
This file uses Time.zone, which is defined in
active_support/core_ext/time/zones.rb.
b3693bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.