You are contradicting yourself. At first you said that it does make the code clearer. Now you say it doesn't. Ok, you guys are right. I'm adding a new feature. If you think my new design isn't good enough do tell me why. I'll improve it/change it completely. It now doesn't violate SRP, it allows flexability for those who need it and maintains backward competitability. What else is missing here? I feel a bit like a broken record here because I did specify over and over what's the purpose of this change. I don't want to load settings from a class.* I'd like to provide extensibiloty points in order to help implemet other workflows that people use.* The purpose of this patch is to:
- Allow developers to implement different workflows for loading settings for different environments (class based settings, different modules, different packages). - Allow developers to load settings from other sources like django-debian<https://launchpad.net/django-debian> does. - Allow developers to change settings as they are collected (for example django-harness <https://bitbucket.org/neithere/django-harness> does multiple things like this). There are usecases, some people actually implemented those outside of the framework and I don't see a reason why they should. We're perfectionists with deadlines right? There's no reason to write django-configurations if in five minutes you can get the same result out of the box. Do you see value in django-configurations, django-debian or django-harness? Then you must see the value in this patch. You can implement all of those in an hour tops including documentation and tests with it. If you don't, just keep using the default behavior. No harm done. What exactly is the maintainance burden here? Instead of saying "Check the DJANGO_SETTINGS_MODULE" you now just need to ask "Have you written a new collector or loader? No? Check the DJANGO_SETTINGS_MODULE". You want to change how settings are being loaded by default? How does that contradict the use of my patch? In fact it makes it much easier because now you can just rewrite the collector and loader and you have a new default way of loading settings. I know there were talks about changing how settings behave and that's why I did something about it. Am I clear enough now? 2013/3/24 Russell Keith-Magee <russ...@keith-magee.com> > Hi Omer, > > I've looked at your code, and while you continue to call it a > "refactoring", it's a refactoring with a specific purpose in mind -- to add > a new feature. > > I disagree that your new code is "clearer" and more "understandable" than > the existing code. For starters, it's got more moving parts (pluggable > Collector and Loader interfaces, for example), and while it's not too hard > to work out how the parts fit together, it isn't intuitively obvious (which > is what would be needed to genuinely have a simplification here). > > On top of all that, what you still haven't done is express is *why* this > new feature is required. You clearly want the ability to load settings from > a class in some way. It isn't obvious to met at all why this is beneficial > or helpful. > > Lastly, the settings module is one of the warts of Django that most of the > core team would like to fix. In this case, "Fixing" it doesn't mean adding > more complexity and/or flexibility. It means altering altogether the way > settings are handled in Django. There hasn't really been any work in this > area beyond some high level talks at DjangoCon etc [1], but adding > additional complexity to Django's already complex settings system isn't > something we're racing to do without a *really* good reason. > > [1] http://www.youtube.com/watch?v=0FD510Oz2e4 > > Yours, > Russ Magee %-) > > On Sat, Mar 23, 2013 at 3:47 PM, Omer Katz <omer.d...@gmail.com> wrote: > >> So is my example good enough? Have you tried using it? >> >> בתאריך יום שישי, 15 במרץ 2013 13:17:11 UTC+3, מאת Aymeric Augustin: >> >>> On 15 mars 2013, at 09:22, Omer Katz <omer...@gmail.com> wrote: >>> >>> > Doesn't the fact that the patch makes the code clearer is a reason >>> enough for a merge (providing that there will be tests attached to it and >>> documentation)? >>> >>> >>> Hi Omer, >>> >>> This patch isn't only a refactoring; it adds a new feature. Otherwise >>> you wouldn't be talking about documentation. >>> >>> Each feature added to Django creates a burden: >>> - for users of Django, who must learn to use it; >>> - for the core team, who must maintain it for the foreseeable future. >>> >>> To be accepted, a new feature must: >>> (a) provide benefits that clearly outweigh these costs >>> (b) not get in the way of future improvements — as much as can be >>> foreseen. >>> >>> Unfortunately (a) the benefits of your PR still aren't clear and (b) >>> judging by the discussion, your abstraction doesn't match very well the >>> needs of most users, and I suspect it could hinder further efforts to make >>> per-environment settings (the actual problem) easier to define. >>> >>> -- >>> Aymeric. >>> >>> -- >> You received this message because you are subscribed to the Google Groups >> "Django developers" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to django-developers+unsubscr...@googlegroups.com. >> >> To post to this group, send email to django-developers@googlegroups.com. >> Visit this group at >> http://groups.google.com/group/django-developers?hl=en. >> For more options, visit https://groups.google.com/groups/opt_out. >> >> >> > > -- > You received this message because you are subscribed to a topic in the > Google Groups "Django developers" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/django-developers/1M5nfnpba0M/unsubscribe?hl=en > . > To unsubscribe from this group and all its topics, send an email to > django-developers+unsubscr...@googlegroups.com. > To post to this group, send email to django-developers@googlegroups.com. > Visit this group at http://groups.google.com/group/django-developers?hl=en > . > For more options, visit https://groups.google.com/groups/opt_out. > > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
