Skip to content

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

Merged
dblock merged 1 commit into
codegram:masterfrom
koenpunt:accept-json-hal
Dec 1, 2014
Merged

Use application/hal+json as default Content-Type and add to Accept header#81
dblock merged 1 commit into
codegram:masterfrom
koenpunt:accept-json-hal

Conversation

@koenpunt
Copy link
Copy Markdown
Contributor

@koenpunt koenpunt commented Nov 30, 2014

Not every api accepts the application/json header.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Nov 30, 2014

Needs tests, CHANGELOG and possible README updates please.

@koenpunt
Copy link
Copy Markdown
Contributor Author

koenpunt commented Nov 30, 2014

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
Copy Markdown
Collaborator

dblock commented Nov 30, 2014

You mean the default content-type? Yes.

@koenpunt
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

dblock commented Dec 1, 2014

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

@koenpunt
change default content-type to application/hal+json
9e81da5 
add application/hal+json to 'Accept' header
add custom middleware for json+hal
@koenpunt
Copy link
Copy Markdown
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
@dblock
Merge pull request #81 from koenpunt/accept-json-hal
39b6c5f 
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
Copy Markdown
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
Copy Markdown
Contributor Author

koenpunt commented Dec 1, 2014

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

@koenpunt koenpunt deleted the accept-json-hal branch December 1, 2014 16:39
@dblock dblock mentioned this pull request Feb 25, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@koenpunt @dblock