Opened 3 years ago

Closed 3 years ago

#20040 closed Cleanup/optimization (needsinfo)

Refactoring of the settings module to be more modular and extensible

Reported by: the_drow Owned by: the_drow
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


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 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 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

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 3 years ago by the_drow

  • Needs documentation set
  • Needs tests set
  • Resolution needsinfo deleted
  • Status changed from closed to new

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 3 years ago by the_drow (next)

comment:3 Changed 3 years ago by claudep

  • Resolution set to needsinfo
  • Status changed from new to closed

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