Skip to content

Add DRF 3.7 and Django 2.0 to the test matrix. #394

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

Merged
merged 8 commits into from
Jan 24, 2018

Conversation

mblayman
Copy link
Collaborator

Fixes #388

@mblayman
Copy link
Collaborator Author

The DJA test suite relies on the example app which, in turn, relies on django-polymorphic. There is an open issue with django-polymorphic (jazzband/django-polymorphic#324) that prevents it from running on Django 2.0.

Once django-polymorphic works on Django 2.0, I can come back to this branch and (hopefully), merge it.

@mblayman mblayman changed the title Add DRF 3.7 to the test matrix. Add DRF 3.7 and Django 2.0 to the test matrix. Jan 7, 2018
@sliverc
Copy link
Member

sliverc commented Jan 9, 2018

Maybe we should think about making the dependency django-polymorphic optional as the majority of users are properly not using it. So if someone wants to use it they already have the dependency set in their own project to create the models and DJA will detect it that it is installed.

This way DJA wouldn't be blocked by issues like this to upgrade to a newer Django and DRF version. The tests in this PR would simply need to exclude polymorphic tests to be run on Django 2.0 till django-polymorphic has fixed it.

@mblayman
Copy link
Collaborator Author

mblayman commented Jan 9, 2018

I considered that route, but ultimately decided against it because I thought it would be too confusing.

If DJA released a version compatible with Django 2.0, but didn't work with django-polymorphic because polymorphic still doesn't work with 2.0, then I feel like the documentation would be lying to users. I'm not comfortable stating DJA supports Django 2.0 and django-polymorphic at the same time.

In addition (and probably the bigger problem), the example app uses a polymorphic model, and I don't know how to conditionally make models in the example app in a way that won't cause migrations oddities. One part of this project that I find puzzling is that the entire test suite is extremely coupled to the example project. I understand that it's for historical reasons, but I think that is the blocker.

I think that the 2.0 branch for polymorphic is blocked (jazzband/django-polymorphic#318) so maybe the best thing we could do is go help them get unblocked. 😄

@sliverc
Copy link
Member

sliverc commented Jan 10, 2018

Fair enough. Not sure whether I will find the time myself but it would properly be a good idea to assist django-polymorphic with 2.0 support.

@mblayman
Copy link
Collaborator Author

Cool, it looks like there was progress on the django-polymorphic issue. I'll keep my eyes open for a new release that has support for Django 2.0.

Older versions of django-polymorphic don't support Django 2.0
@mblayman
Copy link
Collaborator Author

Excellent! Many folks are looking for a Django 2.0 compatible release so I'm going to merge this and make a release in the next couple of days.

@mblayman mblayman merged commit 1bbf6fa into django-json-api:master Jan 24, 2018
@mblayman mblayman deleted the drf-37 branch January 24, 2018 16:50
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