search

bittner / django-apptemplates

Django template loader that allows you to load and override a template from a specific Django application.
https://pypi.python.org/pypi/django-apptemplates
MIT License
51 stars 12 forks source link

Order of imports that have moved #9

Closed dracos closed 6 years ago

dracos commented 6 years ago

Only a tiny thing, but the comment at https://github.com/bittner/django-apptemplates/blob/95b6fcdecf28e63e040f081b3f9cb782b9e18b15/tests.py#L7-L10 implies django.test.utils is the current location, but that is the old location – on https://docs.djangoproject.com/en/1.8/topics/testing/tools/#django.test.override_settings it says: "Changed in Django 1.7: Previously, override_settings was imported from django.test.utils."

I'm not sure why this got swapped round in https://github.com/bittner/django-apptemplates/commit/623842bafe15e0ca5692ffe03a45b6bf479f9b7e#diff-f4bc68ff01eef7bed2f3a64cad7d37ca but the other try import in https://github.com/bittner/django-apptemplates/blob/95b6fcdecf28e63e040f081b3f9cb782b9e18b15/apptemplates/__init__.py#L9-L12 puts the more recent location first, so I thought this should match. But as I say, only a tiny thing :)

bittner commented 6 years ago

I'm deeply sorry, I swapped this. It was mainly for aesthetic reasons after I failed to find the reference you just mentioned. The motivation was exactly what you're mentioning, placing the more likely one first. I looked at the Django source code and found both possibilities present. So I assumed what I then changed made most sense, including adding the comment to explain things.

I'll swap this around immediately. Though, technically, apart from a higher probability that an ImportError is thrown, it shouldn't make any difference, right? Hence, I can issue a new release but technically I wouldn't have to. Luckily it's just the tests, so we don't need a new release.

Sorry again.

dracos commented 6 years ago

No need to apologise, I debated saying anything at all given, as you say, it makes no difference to the running; I just thought it might be the sort of thing I looked back on in a couple of years and got slightly confused :)

Thanks, and for maintaining this app! In case you're interested, we use it on https://mapit.mysociety.org/ so that it can be a reusable Django app at https://github.com/mysociety/mapit but that specific install - source https://github.com/mysociety/mapit.mysociety.org - can override the parent base template easily :)

bittner commented 6 years ago

I'm glad this app is useful in real life!

I'd be happy to have more statistics, it's always interesting to know more about how a package is used. Unfortunately, PyPI doesn't provide download statistics anymore.