Skip to content

Acceptance of malformed relationships #49

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

Closed
chamini2 opened this issue Mar 6, 2018 · 2 comments
Closed

Acceptance of malformed relationships #49

chamini2 opened this issue Mar 6, 2018 · 2 comments

Comments

@chamini2
Copy link
Contributor

chamini2 commented Mar 6, 2018

I noticed that in 62b7386 you are handling malformed relationships by ignoring them. This seems to me a debatable thing to do, since maybe we should be warning the user of the malformed data, rather than ignoring it.

I don't ask for us to check if it's malformed and throw on it, what I would suggest is not check for this at all. JSON API specifies a very clear payload format, so if it is not followed, it's understandable that this package would not work correctly.

What I am trying to say is, why not make the code easier to maintain by accounting for less cases? Maybe I can remove this specific check in #48.

@danivek
Copy link
Owner

danivek commented Mar 6, 2018

I accepted your PR too fast...

I totally agree with you. The best thing to do here is to remove this specific check.

@danivek danivek closed this as completed in 4662768 Mar 6, 2018
@chamini2
Copy link
Contributor Author

chamini2 commented Mar 6, 2018

Great, @danivek, I left a comment: 4662768#r27962123

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

No branches or pull requests

2 participants