Code

Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#21372 closed Bug (fixed)

Documentation on Settings wrong about django.utils.translation

Reported by: salvatore@… Owned by: bernardopires
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: salvatore@…, carneiro.be@…, bmispelon 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.

Attachments (0)

Change History (10)

comment:1 Changed 6 months ago by chrismedrela

  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

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

comment:2 Changed 5 months ago by Bernardo Pires Carneiro <carneiro.be@…>

  • Cc carneiro.be@… added
  • Owner changed from nobody to anonymous
  • Status changed from new to assigned

comment:3 Changed 5 months ago by bernardopires

  • Owner changed from anonymous to bernardopires

comment:4 Changed 5 months ago by bernardopires

  • 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 Changed 5 months ago by bmispelon

  • Cc bmispelon 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 Changed 5 months ago by bernardopires

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 Changed 5 months ago by bmispelon

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to 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 Changed 5 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

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 Changed 5 months ago by Baptiste Mispelon <bmispelon@…>

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 Changed 5 months ago by Baptiste Mispelon <bmispelon@…>

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.