#21372 closed Bug (fixed)
Documentation on Settings wrong about django.utils.translation
Reported by: | Owned by: | Bernardo Pires Carneiro | |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | salvatore@…, carneiro.be@…, Baptiste Mispelon | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The page https://docs.djangoproject.com/en/dev/ref/settings/ says:
You should never import django.utils.translation from within your settings file, because that module in itself depends on the settings, and that would cause a circular import.
That's actually not true: you can use ugettext_lazy with no problems.
Change History (10)
comment:1 by , 11 years ago
Easy pickings: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
comment:2 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 11 years ago
Owner: | changed from | to
---|
comment:4 by , 11 years ago
Has patch: | set |
---|
Alright, since translating the language names doesn't require any workaround using a dummy function, I've made the LANGUAGES
docs more concise and just explained how to use ugettext_lazy
to translate it.
This was my first contribution to django and hopefully I've done everything correctly! Of course I'll gladly review any suggestions.
The patch is available on https://github.com/bernardopires/django/tree/ticket_21372
comment:5 by , 11 years ago
Cc: | added |
---|---|
Patch needs improvement: | set |
The patch is looking good but as mentionned on the pull request, there's another mention of the "dummy" gettext in docs/topics/i18n/translation.txt
that should be removed as well.
I also dug a bit and found the commit when this bit of documentation was added: 93b21610b9decbda74b848a249f739534400f919.
I tested with a really old version of django (1.0) and while it's true that importing ugettext
will trigger a circular import, using ugettext_lazy
works without any issue.
comment:6 by , 11 years ago
Thanks for the fast review! I've now updated docs/topics/i18n/translation.txt
too.
Questions: 1. After I've done the requested changes by a reviewer, where do I notify the reviewer about it, here or on the pull request? 2. Should I unset myself "Patch needs improvent" after I've done the changes?
Thanks in advance for your response!
comment:7 by , 11 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Hi,
Regarding question 1), posting a comment on the ticket like you just did is a good option (I had CC-ed myself on this ticket and got a notification).
Answering to the comments made on the pull request is also good and if all else fails, you can always come on the #django-dev
IRC channel and ask for a review.
For 2), the answer is yes.
I'm marking this ticket as ready for checkin
and I'll be committing it soon.
Thanks for your contribution. I'm looking forward to the next one ;)
comment:8 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Wow, importing
django.utils.translation
seems to be allowed for at least 6 years!