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

Allow valid accept headers in document loader options #182

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lutangar
Copy link
Contributor

@lutangar lutangar commented May 12, 2017

Hi guys,

Here is a feature that was previously mentioned in #176

The current implementation is far too restrictive on this accept header. The RangeError exception should only be raised when the accept header doesn't contain application/ld+json nor application/json.

This way it could be overridden for more exotic things like application/ld+json, text/html, application/json, that would still be valid.

The point here is to allow valid headers to be used, because at the moment, even if the user specify exactly the default value application/ld+json, application/json the RangeError is raised nonetheless.

Also, even if the buildHeaders function wasn't meant to be exposed, at the moment this is only way to unit test portions of code as there are no builds tools to allow to split things up and require them only where needed. By the way, is there any news/plans on this?

Anyway, what do you think of this feature?

@lutangar
Copy link
Contributor Author

@dlongley @davidlehn any feedbacks on this?

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

1 participant