Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#16758 closed Cleanup/optimization (fixed)

Documentation for TEMPLATE_CONTEXT_PROCESSORS could use additional warning.

Reported by: cyclops Owned by: anonymous
Component: Documentation Version: 1.3
Severity: Normal Keywords:
Cc: Yaşar Arabacı 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 documentation for TEMPLATE_CONTEXT_PROCESSORS, at:

https://docs.djangoproject.com/en/1.3/ref/settings/#template-context-processors

could use a warning about over-riding the variable in settings.py, maybe something like:

"Warning - if you set this variable to add your own context processor, it will over-ride the default settings listed above. You must manually add the defaults back into the variable, for them to be available."

Note that this issue has come up and been documented also on StackOverflow: http://stackoverflow.com/questions/2246725/django-template-context-processors

Attachments (4)

0001-Fix-ticket-16758.patch (1.3 KB) - added by Yaşar Arabacı 5 years ago.
Added neccessary warning.
0001-Added-warning-in-settings.txt.patch (1.2 KB) - added by Yaşar Arabacı 5 years ago.
Re-written warnings according to remarks.
0001-docs-ref-setting.txt-added-warning.patch (1.1 KB) - added by Yaşar Arabacı 5 years ago.
16758-4.patch (561 bytes) - added by Aymeric Augustin 5 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 5 years ago by Aymeric Augustin

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Type: BugCleanup/optimization

The situation is identical for all other settings that have a non-empty tuple as default value, notably MIDDLEWARE_CLASSES.

Without a good patch, I'm reluctant to accept this ticket because we can't list all possible errors and this one doesn't look too hard to diagnose.

Last edited 5 years ago by Aymeric Augustin (previous) (diff)

comment:2 Changed 5 years ago by Julien Phalip

Triage Stage: UnreviewedAccepted

This could be a useful clarification for beginners, but since it applies to all settings it should be a global note at the top of the page.

comment:3 Changed 5 years ago by Yaşar Arabacı

Owner: changed from nobody to Yaşar Arabacı
Status: newassigned

Changed 5 years ago by Yaşar Arabacı

Attachment: 0001-Fix-ticket-16758.patch added

Added neccessary warning.

comment:4 Changed 5 years ago by Yaşar Arabacı

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:5 Changed 5 years ago by Yaşar Arabacı

Cc: Yaşar Arabacı added

comment:6 Changed 5 years ago by Aymeric Augustin

Triage Stage: Ready for checkinAccepted

Please don't mark your own patches as RFC — the rule is that patches must be reviewed by someone who isn't the author. For more information see the contributing guide in the docs.

Documentation patches are reviewed on a regular basis, so don't worry, your patch will get attention eventually.

comment:7 Changed 5 years ago by Bas Peschier

Patch needs improvement: set

Thanks for your contribution; I do have a couple of remarks on the patch:

  1. The suggested text focuses on what people should do instead of WHY they should do it, which feels to me to be belittling the reader a bit.
  2. It's not only beginners who should be cautious, everybody should.
  3. I believe Django is commonly referred to with a capital D.

I would suggest to rewrite it to a more general style ("be careful with settings in settings.py, because ...") and focus on the fact settings.py overrides the default settings.

Changed 5 years ago by Yaşar Arabacı

Re-written warnings according to remarks.

comment:8 in reply to:  6 Changed 5 years ago by Yaşar Arabacı

Replying to aaugustin:

Please don't mark your own patches as RFC — the rule is that patches must be reviewed by someone who isn't the author.

Sorry, I didn't know. I am new to contributing to django.

Replying to bpeschier:

Thanks for your contribution; I do have a couple of remarks on the patch:

Thanks for the remarks. I attached a new patch file.

comment:9 Changed 5 years ago by Christopher Medrela

The beginning of the warning is:

Be careful with the settings in settings.py, because settings in settings.py override the default values. Overriding some settings may cause ...

Word 'settings' is repeated and repeated. I suggest changing it to something like this

Be careful when you override default values of settings, because it may cause ...


Next, there is

... may cause some core or commonly used functions of Django to stop functioning ...

I'm not sure 'functioning' is good word. I prefer simpler words, for example:

... may cause some core or commonly used functions of Django to stop working correctly ...


yasar11732, you wrote

When overriding default values, you are advised to make sure that none of the functions of Django that you need requires previous setting to work correctly.

Are there features of Django that cannot work correctly with non default values of some settings? I'm asking because I'm new contributor and i don't know everything about Django.


If the setting you are trying to change consists of more than one value (e.g a tuple), ...

I suggest

If the setting you want to change is tuple, list or dictonary, ...

because there are more words 'want' than 'try' in tutorial. I'm not sure if 'is tuple, list or directory' is more clear than 'consists of more than one value'.

Changed 5 years ago by Yaşar Arabacı

comment:10 Changed 5 years ago by Yaşar Arabacı

Sorry for late response, I was busy lately. How about latest patch? And about your question, some features need some TEMPLATE_CONTEXT_PROCESSORS to be loaded for example.

comment:11 Changed 5 years ago by Christopher Medrela

Triage Stage: AcceptedReady for checkin

comment:12 Changed 5 years ago by Aymeric Augustin

Patch needs improvement: unset

I'm sorry, but I still feel this doesn't match very well the tone of the docs. I'm uploading a new proposal.

Changed 5 years ago by Aymeric Augustin

Attachment: 16758-4.patch added

comment:13 Changed 5 years ago by anonymous

Owner: changed from Yaşar Arabacı to anonymous
Status: assignednew

comment:14 Changed 5 years ago by Yaşar Arabacı

I have disowned ticket. Someone else might own it if needed.

comment:15 Changed 5 years ago by Claude Paroz

Aymeric, I like your version.

comment:16 Changed 5 years ago by Tim Graham

Resolution: fixed
Status: newclosed

In [17566]:

Fixed #16758 - Added a warning regarding overriding default settings; thanks cyclops for the suggestion & Aymeric Augustin for the patch.

comment:17 Changed 5 years ago by Tim Graham

In [17567]:

[1.3.X] Fixed #16758 - Added a warning regarding overriding default settings; thanks cyclops for the suggestion & Aymeric Augustin for the patch.

Backport of r17566 from trunk.

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