search

paulocheque / django-dynamic-fixture

A complete library to create dynamic model instances for testing purposes.
http://django-dynamic-fixture.readthedocs.io/
Other
391 stars 67 forks source link

Django 3.0 field choices may be None. #97

Closed jheld closed 4 years ago

jheld commented 4 years ago

Bugfix around a behavior/code flow change from Django 3.0.

Turns out due to some of the changes around the 2.x and 3.x series if we want to keep general support (and look to the future), we needed more changes around the interpreters, django versions, and the test setup/assertions. Nothing tremendous -- so long as the tests end up passing, I think beyond it being a version update we should be fine with this.

Currently failing around GDAL installation issues.

jheld commented 4 years ago

@paulocheque Can you check into these changes? There isn't much code/library change.

I'm only somewhat familiar with the GDAL dependency, but none of it seems related to these changes.

paulocheque commented 4 years ago

Hey @jheld , awesome contribution! I kind of get blocked in the same issue and created a simpler solution, but maybe yours focus for another scenarios.

Could you rebase with master and confirm to me if that still makes sense?

Also, duplicate of https://github.com/paulocheque/django-dynamic-fixture/pull/98

paulocheque commented 4 years ago

@jheld I fixed the GDAL issues locally and in Travis. Thanks!

paulocheque commented 4 years ago

@jheld I kind of use models.DO_NOTHING instead of models.CASCADE in the on_delete. What do you think?

jheld commented 4 years ago

@paulocheque thanks for working through these and other issues lately. It's great to see that DDF is catching up.

DO_NOTHING is fine. I prefer CASCADE but it's fine.

What of this PR would you still like merged?

paulocheque commented 4 years ago

@jheld

Do you have any good usage for the CASCADE? I never care about deleting objects in tests with DDF, because it is done automatically via tearDown.

If you think it is worth to make a cascade, we can adapt the PR to submit just that change.

Otherwise, we can close this one.

Thanks again!

jheld commented 4 years ago

@paulocheque It's just my preference. But I think django 2+ requires we specify an on_delete value, so whether it's this PR or not we do need to make a change.

paulocheque commented 4 years ago

@jheld The on_delete=DO_NOTHING is already on master. So I will close this PR for now.