Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update blog_article active_support requires. Fixes #205 #206

Merged
merged 1 commit into from May 2, 2014

Conversation

adamesque
Copy link
Contributor

As far as I can tell, requiring activesupport/core_ext/time/acts_like keeps us from taking a bad branch at https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/date_and_time/zones.rb#L21 which leads to an exception when calling to_time with a parameter.

I wonder, though, if it might be easier just to include all the ActiveSupport Time core_exts rather than cherry-pick at this granularity.

@tdreyno
Copy link
Member

tdreyno commented Apr 10, 2014

I think it is better to be explicit. Ruby already has slow startup time and a lot of that is just reading files off disk. Avoiding "include everything" scenarios, especially with ActiveSupport which monkey patches everything, should be avoided.

Thanks for tracking this down.

@adamesque
Copy link
Contributor Author

Yeah, that makes sense. This fix actually kinda sucks because we aren't even calling acts_like_time? directly. This has got to be an ActiveSupport bug, but I have no idea exactly where.

@Arcovion
Copy link

👍

@tdreyno
Copy link
Member

tdreyno commented Apr 11, 2014

@adamesque It does seem like a missing dependency on the activesupport side.

bhollis added a commit that referenced this pull request May 2, 2014
Update blog_article active_support requires. Fixes #205
@bhollis bhollis merged commit bb36683 into middleman:master May 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants