Skip to content

Conversation

jaylett
Copy link

@jaylett jaylett commented May 1, 2015

  • use staticfiles to generate asset URLs properly
  • drop various pre-1.6 support to cope with removals in 1.6..1.8
  • support 1.5+ custom user models, including in migrations
  • south migrations for 1.6, native migrations for 1.7
  • a single test from Texas Tribune work, and a simple way to run them
  • package can be make and installed using sdist
  • documentation perhaps a little closer to what actually happens

jaylett and others added 22 commits May 1, 2015 09:42
Based on npinchot’s PR to RobCombs, but with BC from the
Django release notes for 1.6.
Otherwise you can’t reliably use more sophisticated features
of the storage backends, like hashed URLs as implemented by
CachedFilesMixin.

Since you should be either using that approach, or prefixing
your static fileset with something unique per release (say
a version number or the git SHA) then we drop the “?v=6”
version suffix on our Javascript at the same time.
Changed 'mimetype' param to 'content_type’; this is
supported from Django 1.5, so removes support for 1.4 and
earlier.
Since we don’t ship README.md, we can’t rely on it in
setup.py or sdists won’t actually install. Which is bad.
 * can’t override time_until_expiration setting because locking
   hides its actual settings away (and we can’t use @override_settings)
   consequently just test against the default
 * move test factories somewhere sensible
 * timezone-aware datetimes
In preparation for creating Django 1.7 migrations for this app.
Otherwise the app registry won’t be ready, and probably some other
things.
Also include manage.py & manage-specific settings file, just for
making migrations. Feels a bit grotty, but it’s a pain to make
migrations otherwise.
1.7 migration looks like it’ll work out of the box (less surprisingly).

Credit for approach: http://kevindias.com/writing/django-custom-user-models-south-and-reusable-apps/
This is part of the 1.5+ custom user model API. .username is not.
@jaylett
Copy link
Author

jaylett commented May 1, 2015

Didn't actually mean to include the Grappelli commit at the end, but it should be safe for everyone.

@fdintino
Copy link
Member

fdintino commented Oct 1, 2015

There is a lot of good stuff in here, but unfortunately I did django compatibility changes without looking here first. So now there are merge conflicts. I'll see if I can't cherry pick some of the changes unrelated to django compatibility at some point. (Unless you wanted to open them as separate, more atomic pull requests, which would be awesome).

@jaylett
Copy link
Author

jaylett commented Oct 1, 2015

Yes, sorry, it's an awful grab bag of stuff. The app that uses this isn't under active maintenance, so I have no idea when I'll next get a chance to look at this properly, but the vast, vast majority of changes were for compatibility, so cherry-picking other commits (or even just reimplementing them, except for the Texas Tribune work) may turn out to be easier. If I get a chance I'll try to put together discrete PRs!

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.

3 participants