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

Use application/hal+json as default Content-Type and add to Accept header #81

Merged
merged 1 commit into from
Dec 1, 2014
Merged

Use application/hal+json as default Content-Type and add to Accept header #81

merged 1 commit into from
Dec 1, 2014

Conversation

koenpunt
Copy link
Contributor

Not every api accepts the application/json header.

@dblock
Copy link
Collaborator

dblock commented Nov 30, 2014

Needs tests, CHANGELOG and possible README updates please.

@koenpunt
Copy link
Contributor Author

I think the Content-Type should be application/hal+json as well, what do you think?

At least according to the spec: https://github.com/mikekelly/hal_specification/blob/master/hal_specification.md#how-to-serve-hal

@dblock
Copy link
Collaborator

dblock commented Nov 30, 2014

You mean the default content-type? Yes.

@koenpunt
Copy link
Contributor Author

koenpunt commented Dec 1, 2014

I'll update the CHANGELOG in a bit

@koenpunt koenpunt changed the title add application/hal+json to 'Accept' header Use application/hal+json as default Content-Type and Accept headers Dec 1, 2014
@koenpunt koenpunt changed the title Use application/hal+json as default Content-Type and Accept headers Use application/hal+json as default Content-Type and add to Accept header Dec 1, 2014
@koenpunt
Copy link
Contributor Author

koenpunt commented Dec 1, 2014

For now tagged it as 0.7.0-pre, but I think we can maybe combine this with #80. Because if we're going break the API we better break it good :)

@dblock
Copy link
Collaborator

dblock commented Dec 1, 2014

This is good, would you mind rebasing and squashing your commits please?

add application/hal+json to 'Accept' header
add custom middleware for json+hal
@koenpunt
Copy link
Contributor Author

koenpunt commented Dec 1, 2014

Done, and what do you think of my suggestion in #80?

dblock added a commit that referenced this pull request Dec 1, 2014
Use application/hal+json as default Content-Type and add to Accept header
@dblock dblock merged commit 39b6c5f into codegram:master Dec 1, 2014
@dblock
Copy link
Collaborator

dblock commented Dec 1, 2014

I did comment on #80, I would take the change, just needs README, CHANGELOG, tests, etc.

@koenpunt
Copy link
Contributor Author

koenpunt commented Dec 1, 2014

Yeah I know, I meant to wait with the release because it could break things.

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.

2 participants