Skip to content

Remove .to_json from renderer #56

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

Merged
merged 3 commits into from
May 15, 2017
Merged

Remove .to_json from renderer #56

merged 3 commits into from
May 15, 2017

Conversation

vasilakisfil
Copy link
Contributor

Renderer returns a hash instead, fixes #55

@codecov-io
Copy link

codecov-io commented Mar 17, 2017

Codecov Report

Merging #56 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #56   +/-   ##
=======================================
  Coverage   99.55%   99.55%           
=======================================
  Files          17       17           
  Lines         453      453           
=======================================
  Hits          451      451           
  Misses          2        2
Impacted Files Coverage Δ
lib/jsonapi/serializable/renderer.rb 92.85% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f965b3...55c84a5. Read the comment docs.

@beauby
Copy link
Member

beauby commented Mar 21, 2017

@vasilakisfil Quick update: the reason I'm holding off merging this is simply that it's a breaking API change (though it should only impact people who use jsonapi-rb as a standalone ruby lib), and there are a few non-breaking changes that I'd like to release before 0.2.

@vasilakisfil
Copy link
Contributor Author

@beauby makes sense to me 👍

@Aryk
Copy link

Aryk commented Apr 13, 2017

just curious, when do you think this will make it into repo? Perhaps in the meantime, we can do

render_hash.to_json

That way you won't break the api, but people can access the hash directly?

@beauby
Copy link
Member

beauby commented Apr 24, 2017

@Aryk Sorry for the late reply – this should make it in tomorrow.

@beauby
Copy link
Member

beauby commented Apr 24, 2017

@vasilakisfil Would you mind rebasing?

Edit: Nevermind, this new GH feature for fixing merge conflicts is awesome.

@beauby
Copy link
Member

beauby commented Apr 24, 2017

Hi @vasilakisfil – would you mind fixing the new test that's broken by this PR? (Basically remove the JSON.parse() call in ./spec/renderer/namespace_spec.rb:13.

@vasilakisfil
Copy link
Contributor Author

Sure, I can do it later today!

@vasilakisfil
Copy link
Contributor Author

@beauby fixed!

@beauby
Copy link
Member

beauby commented May 15, 2017

@vasilakisfil Finally merging – thanks!

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.

There should be a public API that allows to render in a hash.
4 participants