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

active_support/core_ext breaks JSON output, is it really needed? #28

Merged

Conversation

glennpratt
Copy link
Contributor

If duration is required in any of our projects it changes our JSON time formats, breaking our API. This is because of ActiveSupport::CoreExtensions:

http://api.rubyonrails.org/files/activesupport/lib/active_support/core_ext/object/json_rb.html

This is pretty bad behavior in my opinion. Are active_support/core_ext's really needed? I don't think it's a good idea for libraries to use them.

@glennpratt
Copy link
Contributor Author

I converted this to a PR which just pushes the issue over to mongodb/mongoid. Since I don't use mongodb, that solves the issue for me.

I'm not sure why Travis-CI is failing, I don't think it's any change I made. I tried limiting iso8601 to a version that works on 1.9 and jruby since that was causing some fails.

Ping @peleteiro.

@peleteiro
Copy link
Collaborator

@glennpratt it's been quite a while I don't use ruby. I'm trying to find another owner for this project. Are you interested?

Meanwhile I'll try to take a look on this over the next weekend. Thanks.

@glennpratt
Copy link
Contributor Author

@peleteiro I'd be happy to help maintain, though I can't guaranty response times or anything. 😄

https://rubygems.org/profiles/glennpratt

@glennpratt glennpratt force-pushed the active-support-json-issues branch 2 times, most recently from c2dc211 to dff2cf8 Compare March 1, 2016 04:29
@glennpratt
Copy link
Contributor Author

@peleteiro seems like the issue was the version of bundler on different Travis Rubies. I added a gem install bundler and tests pass.

glennpratt added a commit that referenced this pull request Mar 1, 2016
active_support/core_ext breaks JSON output, is it really needed?
@glennpratt glennpratt merged commit f3e508c into josedonizetti:master Mar 1, 2016
@glennpratt
Copy link
Contributor Author

Thanks @peleteiro

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.

2 participants