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 markdown and PyYAML to tox test requirements #2133

Merged
merged 2 commits into from
Nov 25, 2014
Merged

Add markdown and PyYAML to tox test requirements #2133

merged 2 commits into from
Nov 25, 2014

Conversation

jpadilla
Copy link
Member

Don't think there was a reason for not having markdown and PyYAML added to the test dependencies in tox.in. Figured it must have been forgotten a while back.

There are tests failing on Python 3 and the YAML support, but probably not many have noticed , my guess is not many people actually using YAML with DRF and tests being skipped because of missing requirements.

I'm still not sure how to correctly fix the issue, but I think the YAMLRenderer should be returning a string not bytes in Python 3.

As per this comment, I also started extracting the YAML parser and renderer as a third party package.

https://github.com/jpadilla/django-rest-framework-yaml

@tomchristie
Copy link
Member

@jpadilla
Copy link
Member Author

@tomchristie will test that as a possible fix. Open a new PR if it works?

@tomchristie
Copy link
Member

As part of this is fine.

@jpadilla
Copy link
Member Author

So after a while struggling with this, I managed to fix the failing tests without having to change the renderer or parser. Still not sure if that's correct.

@tomchristie
Copy link
Member

Looks right to me.

tomchristie added a commit that referenced this pull request Nov 25, 2014
Add markdown and PyYAML to tox test requirements
@tomchristie tomchristie merged commit eeb07ad into encode:master Nov 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants