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

Add response status for POST with inexistent related resource. #1036

Merged
merged 3 commits into from Sep 28, 2016

Conversation

@beauby
Copy link
Contributor

@beauby beauby commented Apr 27, 2016

related: #1033

@dgeb
Copy link
Member

@dgeb dgeb commented Aug 5, 2016

I agree that this should be spelled out under creating resources, as it is under updating resources (see http://jsonapi.org/format/#crud-updating-responses-404).

We need to decide if this should land in 1.0, 1.1, or both.

@beauby
Copy link
Contributor Author

@beauby beauby commented Sep 28, 2016

Anything more I can do to help on this one?

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

@beauby I've labeled this as needing further discussion because of the MUST.

@beauby
Copy link
Contributor Author

@beauby beauby commented Sep 28, 2016

Fair enough – my understanding was that this was highly implied by the spirit of the spec, but forgotten to be clearly spelled out.

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

I agree. I think this change is entirely consistent with the "updating resources" section, and I am not really in favor of downgrading the MUST because its omission was just an oversight.

@ethanresnick
Copy link
Member

@ethanresnick ethanresnick commented Sep 28, 2016

I trust @dgeb's judgement re adding the MUST.

Re landing in 1.0, 1.1 or both...to date, we've been pretty loose about backporting things to 1.0, and I don't think that's been a problem. If we want to get stricter at some point about the published versions being immutable artifacts, we can, but I'm not sure what the value of that would be now.

Regardless, @beauby please update this pr so that it changes the normative-statements.json files as well. (See e.g. #1102.)

@beauby
Copy link
Contributor Author

@beauby beauby commented Sep 28, 2016

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

@beauby please update _format/1.1/index.md as well

@beauby beauby force-pushed the beauby:404-create-missing-related branch from c3f6671 to 8cd108e Sep 28, 2016
@beauby
Copy link
Contributor Author

@beauby beauby commented Sep 28, 2016

@dgeb Done as well.

@dgeb
Copy link
Member

@dgeb dgeb commented Sep 28, 2016

@beauby Thanks again. LGTM.

@ethanresnick Please review.

@ethanresnick
Copy link
Member

@ethanresnick ethanresnick commented Sep 28, 2016

LGTM too. Merged! Thanks @beauby

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.