Opened 4 years ago

Closed 4 years ago

#20040 closed Cleanup/optimization (needsinfo)

Refactoring of the settings module to be more modular and extensible

Reported by: Omer Katz Owned by: Omer Katz
Component: Core (Other) Version: master
Severity: Normal Keywords: settings
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I refactored the settings module to be much more modular and extensible.
It is now possible to switch a Setting class much more easily by injecting it into the LazySettings object's constructor.
This change also introduces the new SettingsCollector class which allows you to control exactly how the settings are being collected from the setting.py module.
It is possible to inject the SettingsCollector class to the Settings class through the constructor as well.
The only thing that is missing in this patch is a method to switch the settings class. That I don't really have an idea how to implement correctly.
This patch oviously needs tests which I didn't have time to write (I spent a full day working on this) and documentation.

Change History (3)

comment:1 Changed 4 years ago by Aymeric Augustin

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Resolution: needsinfo
Status: newclosed

Thank you for trying to contribute to Django. Unfortunately, writing an undocumented patch isn't the best start. Reading the ticket, I cannot even tell which problem you're trying to solve!

Before starting a major refactoring of a critical part of Django, you should discuss it on django-developers, to ensure that you're tackling a real problem and that your solution will be considered for inclusion in Django. Otherwise it's difficult to take advantage of your work.

Besides, I think you forgot to attach the patch :)

comment:2 Changed 4 years ago by Omer Katz

Needs documentation: set
Needs tests: set
Resolution: needsinfo
Status: closednew

I was gonna post the pull request. Here it is.
I already wrote this because I needed it. The settings module is not modular and extendable enough so I rewrote it.

Version 0, edited 4 years ago by Omer Katz (next)

comment:3 Changed 4 years ago by Claude Paroz

Resolution: needsinfo
Status: newclosed

Please follow the advice you've been given and discuss the issue on django-developers mailing list before reopening the ticket.

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