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

Remove SerializableResource.serialize in favor of .new #1129

Merged

Conversation

bf4
Copy link
Member

@bf4 bf4 commented Sep 9, 2015

Per discussion in #1098

@bf4
Copy link
Member Author

bf4 commented Sep 9, 2015

Failures on appveyour are race conditions we haven't fixed yet

 1) Failure:
ActionController::Serialization::ImplicitSerializerTest#test_cache_expiration_on_update [C:/projects/active-model-serializers/test/action_controller/serialization_test.rb:394]:
--- expected
+++ actual
@@ -1 +1 @@
-"{\"id\":1,\"title\":\"ZOMG a New Post\",\"body\":\"Body\",\"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}],\"blog\":{\"id\":999,\"name\":\"Custom blog\"},\"author\":{\"id\":1,\"name\":\"Joao Moura.\"}}"
+"{\"id\":1,\"title\":\"New Post\",\"body\":\"Body\",\"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}],\"blog\":{\"id\":999,\"name\":\"Custom blog\"},\"author\":{\"id\":1,\"name\":\"Joao Moura.\"}}"

@NullVoxPopuli
Copy link
Contributor

👍

@beauby
Copy link
Contributor

beauby commented Sep 15, 2015

Should we put a deprecation warning until 0.10 is released, rather than suppressing the method?

@joaomdmoura
Copy link
Member

I don't think we needed the warning because it was not even on the RC2, but I'm not against it if you think we should.
But I think we should update/add some docs about it on our new docs for sure.

@beauby
Copy link
Contributor

beauby commented Sep 15, 2015

Oh, right. Let's just remove it then.

@NullVoxPopuli
Copy link
Contributor

@bf4 are you finished with this?

NullVoxPopuli added a commit that referenced this pull request Sep 15, 2015
Remove SerializableResource.serialize in favor of `.new`
@NullVoxPopuli NullVoxPopuli merged commit 610775a into rails-api:master Sep 15, 2015
@bf4 bf4 deleted the remove_serializable_resource_serialize branch September 16, 2015 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants