Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

Latest JSONAPI and JSONAPI Reader #49

Closed
danelowe opened this issue Mar 4, 2015 · 6 comments
Closed

Latest JSONAPI and JSONAPI Reader #49

danelowe opened this issue Mar 4, 2015 · 6 comments

Comments

@danelowe
Copy link
Collaborator

danelowe commented Mar 4, 2015

Now that JSONAPI is nearing a 1.0, it might be a good time to start migrating to the latest version.

I've started working on a Reader for JSONAPI to allow for deserialization, and in the process made a few changes to the Format class to bring it closer to the current JSONAPI format (the newer format seems to be a bit easier to manage anyway).

https://github.com/danelowe/yaks/tree/feature/jsonapi_reader

Looking for feedback on what else is left to do, and how to go about it before completing and submitting a pull request.

@plexus
Copy link
Owner

plexus commented Mar 4, 2015

Thanks for working on this. I'm aware the spec has changed quite a bit, but since I personally don't use JSON-API any more I had no incentive to work on it.

It seems that whatever you have is an improvement. I suggest you open a pull request, and we can move the discussion there. This makes it a lot easier to give feedback on the code.

General things that are easily overlooked are updating the README and CHANGELOG. For the latter just add a description under the "master" heading at the top, beneath the link to the diff.

It would also be a good opportunity to check if test coverage on these bits is up to date. We had 100% mutation coverage at one point, but unfortunately I have to admit that has slipped quite a bit nowadays. To run mutant against a certain class you can do PATTERN='Yaks::Format::JsonApi*' bundle exec rake mutant

If you're new to mutant there's an introduction here: http://www.sitepoint.com/mutation-testing-mutant/

@danelowe
Copy link
Collaborator Author

Hey @plexus

Hoping you could help me with two things.

  1. I probably can't do all the work in one go to conform to the latest JSONAPI. Should I simply create PRs to master for each chunk, or wait for a branch, or hold off until I have everything done in one PR
  2. I've got the mutant tests for that namespace to ~99.5%. Have you got any strategies for dealing with the last one? The second parameter would be part of the expected method signature, but isn't used for this formatter yet.
- rspec:5:./spec/acceptance/json_shared_examples.rb:7/Yaks::Format::JsonAPI
- rspec:6:./spec/acceptance/json_shared_examples.rb:18/Yaks::Format::JsonAPI
- rspec:109:./spec/unit/yaks/format/json_api_spec.rb:8/Yaks::Format::JsonAPI with no subresources should not include a "linked" key
- rspec:110:./spec/unit/yaks/format/json_api_spec.rb:29/Yaks::Format::JsonAPI with both a "href" attribute and a self link should give preference to the href attribute
- rspec:111:./spec/unit/yaks/format/json_api_spec.rb:51/Yaks::Format::JsonAPI with a self link should use the self link in output
- rspec:112:./spec/unit/yaks/format/json_api_spec.rb:74/Yaks::Format::JsonAPI with subresources should include links and linked
- rspec:113:./spec/unit/yaks/format/json_api_spec.rb:99/Yaks::Format::JsonAPI with null subresources should not include links
- rspec:114:./spec/unit/yaks/format/json_api_spec.rb:118/Yaks::Format::JsonAPI with no subresources or links should not include links
evil:Yaks::Format::JsonAPI#call:/Users/Dane/Code/goodnest/_gems/community/yaks/yaks/lib/yaks/format/json_api.rb:10:8975d
@@ -1,12 +1,12 @@
-def call(resource, env = {})
+def call(resource, env = nil)
   main_collection = resource.seq.map(&method(:serialize_resource))
   { data: main_collection }.tap do |serialized|
     linked = resource.seq.each_with_object([]) do |res, array|
       serialize_linked_subresources(res.subresources, array)
     end
     unless linked.empty?
       serialized.merge!(linked: linked)
     end
   end
 end

@plexus
Copy link
Owner

plexus commented Mar 12, 2015

In this case I would change the signature to

def call(resource, _env = nil)

The underscore signals it's ignored. Then add a test to make sure it doesn't blow up when you pass it two arguments.

You're welcome to just add PR's for the chunks that you have. I'm assuming it will already be an improvement. You can add a few words to the relevant section in the README about the state of the implementation.

@steveklabnik
Copy link

We just hit RC3, so you all know :)

json-api/json-api#484

@plexus
Copy link
Owner

plexus commented Mar 16, 2015

@steveklabnik Thanks for the heads up :)
On Mar 16, 2015 11:08 PM, "Steve Klabnik" [email protected] wrote:

We just hit RC3, so you all know :)

json-api/json-api#484 json-api/json-api#484


Reply to this email directly or view it on GitHub
#49 (comment).

@steveklabnik
Copy link

🎊

@plexus plexus closed this as completed Apr 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants