Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21372 closed Bug (fixed)

Documentation on Settings wrong about django.utils.translation

Reported by: salvatore@… 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 chrismedrela, 11 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Wow, importing django.utils.translation seems to be allowed for at least 6 years!

comment:2 by Bernardo Pires Carneiro <carneiro.be@…>, 11 years ago

Cc: carneiro.be@… added
Owner: changed from nobody to anonymous
Status: newassigned

comment:3 by Bernardo Pires Carneiro, 11 years ago

Owner: changed from anonymous to Bernardo Pires Carneiro

comment:4 by Bernardo Pires Carneiro, 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 Baptiste Mispelon, 11 years ago

Cc: Baptiste Mispelon 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 Bernardo Pires Carneiro, 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 Baptiste Mispelon, 11 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady 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 Baptiste Mispelon <bmispelon@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 8bc350b38516d8c3a14aed113dd3402b9375b75c:

Fixed #21372 -- Corrected docs regarding translating LANGUAGES.

Corrected LANGUAGES documentation on how to translate language
names. Now using django.utils.translation.ugettext_lazy instead
of a dummy gettext() function.

Thanks to Salvatore for the report.

comment:9 by Baptiste Mispelon <bmispelon@…>, 11 years ago

In 4aed1ee339c4a36035e1fd1a02b1ec9142feac07:

[1.6.x] Fixed #21372 -- Corrected docs regarding translating LANGUAGES.

Corrected LANGUAGES documentation on how to translate language
names. Now using django.utils.translation.ugettext_lazy instead
of a dummy gettext() function.

Thanks to Salvatore for the report.

Backport of 8bc350b38516d8c3a14aed113dd3402b9375b75c from master.

comment:10 by Baptiste Mispelon <bmispelon@…>, 11 years ago

In 12ff1623d63046b9e27f7f8b9fe20750106889ca:

[1.5.x] Fixed #21372 -- Corrected docs regarding translating LANGUAGES.

Corrected LANGUAGES documentation on how to translate language
names. Now using django.utils.translation.ugettext_lazy instead
of a dummy gettext() function.

Thanks to Salvatore for the report.

Backport of 8bc350b38516d8c3a14aed113dd3402b9375b75c from master.

Conflicts:

docs/topics/i18n/translation.txt

Note: See TracTickets for help on using tickets.
Back to Top