django-wiki / django-nyt

Notification system for Django with batteries included: Email digests, user settings, JSON API
Apache License 2.0
144 stars 47 forks source link

South requirement should be > 1.0 #9

Closed spookylukey closed 9 years ago

spookylukey commented 9 years ago

Older versions of South do not work, because they will attempt to import from the 'migrations' folder instead of 'south_migrations'

benjaoming commented 9 years ago

According to the docs, it was introduced in South 0.7:

http://south.readthedocs.org/en/latest/settings.html#south-migration-modules

@valberg any reason for the specific pinning? I think it hinders playing nicely with other applications and their deps... also, when South 1.0.2 comes out, we'd have to change it, which seems a bit annoying to have to do with all apps. AFAIK, pinning is more a case of specific projects and their deployments?

valberg commented 9 years ago

I've pinned it to the latest version since it's the one that looks for south_migrations per default (see http://south.readthedocs.org/en/latest/releasenotes/1.0.html#library-migration-path). Telling users of Django <1.7 to explicitly set it in settings is fine by me though.

benjaoming commented 9 years ago

Yeah, probably the best thing to do would be to get a dynamic setup.py as with this one, which @spookylukey actually made the modifications of :)

https://github.com/django-wiki/django-wiki/blob/master/setup.py