Opened 13 years ago

Closed 12 years ago

Last modified 12 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ı 12 years ago.
Added neccessary warning.
0001-Added-warning-in-settings.txt.patch (1.2 KB ) - added by Yaşar Arabacı 12 years ago.
Re-written warnings according to remarks.
0001-docs-ref-setting.txt-added-warning.patch (1.1 KB ) - added by Yaşar Arabacı 12 years ago.
16758-4.patch (561 bytes ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 by Aymeric Augustin, 13 years ago

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 13 years ago by Aymeric Augustin (previous) (diff)

comment:2 by Julien Phalip, 13 years ago

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 by Yaşar Arabacı, 12 years ago

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

by Yaşar Arabacı, 12 years ago

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

Added neccessary warning.

comment:4 by Yaşar Arabacı, 12 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:5 by Yaşar Arabacı, 12 years ago

Cc: Yaşar Arabacı added

comment:6 by Aymeric Augustin, 12 years ago

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 by Bas Peschier, 12 years ago

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.

by Yaşar Arabacı, 12 years ago

Re-written warnings according to remarks.

in reply to:  6 comment:8 by Yaşar Arabacı, 12 years ago

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 by Christopher Medrela, 12 years ago

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'.

by Yaşar Arabacı, 12 years ago

comment:10 by Yaşar Arabacı, 12 years ago

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 by Christopher Medrela, 12 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Aymeric Augustin, 12 years ago

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.

by Aymeric Augustin, 12 years ago

Attachment: 16758-4.patch added

comment:13 by anonymous, 12 years ago

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

comment:14 by Yaşar Arabacı, 12 years ago

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

comment:15 by Claude Paroz, 12 years ago

Aymeric, I like your version.

comment:16 by Tim Graham, 12 years ago

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 by Tim Graham, 12 years ago

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