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 client_accepts_jsonapi property to Request #585

Closed
wants to merge 1 commit into from

Conversation

smcclstocks
Copy link

I’ll probably have a few PR’s as I develop my own JSONAPI serializer/deserializer. This first one is relatively simple.

For more info on JSON API see http://jsonapi.org/format/.

Thanks & I look forward to contributing!

Simple check to determine if the client is willing to receive JSON API
compliant data.
@Weyzu
Copy link

Weyzu commented Aug 12, 2015

Adding ./.ropeproject directory seems unnecessary.

@smcclstocks
Copy link
Author

Yes I agree & don't know why that is in there. I must have totally overlooked it in my review. I'm not great with git so how would you like me to proceed? Should I refork falcon & make my changes then pull request again? Thanks.

@Weyzu
Copy link

Weyzu commented Aug 12, 2015

I'd do git reset HEAD~, commit things properly and then git push -f (Usually you should avoid force pushing).

Remember to follow the style guide for commit messages. Falcon uses AngularJS's style guide. More info here.

Don't forget to add tests of accepting JSON API format. A scenario can be added in tests/test_req_vars.py::test_client_accepts() although don't take my word for it, since I don't know if further changes should be made. :)

@@ -293,6 +295,10 @@ def client_accepts_json(self):
return self.client_accepts('application/json')

@property
def client_accepts_jsonapi(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to decide how far we should go down the hypermedia path; there are a bunch of media types out there we would have to consider (HAL, Siren, Cj, JSON API, etc.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine & I can understand it. I just passed in my own request object & that has plenty of flexibibility for what I need.

@kgriffs kgriffs closed this Dec 17, 2015
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.

None yet

3 participants