You’re pretty right, some design choices sound really bad when put like this. I’ll move the admin classes, the code_list and the frontend language selector somewhere else.
As for the TRANSLATED setting, I don’t recall exactly why I tied it with USE_I18N. If I’m correct, it is because django-modeltranslation deactivates itself when USE_I18N is False. So we can’t relly solely on USE_MODELTRANSLATION to know when translations are activated. I’ll check that by tomorow and let you know. But if it is the case, what is your suggested approach ? Force the USE_I18N to True if USE_MODELTRANSLATION is True ? Tighten TRANSLATED into settings instead of a global var ? Le samedi 7 juin 2014 12:09:02 UTC+2, Stephen McDonald a écrit : > > Ok I spent a bit of time going over the pull request and testing it, it's > really good. > > I think we can tighten things up a little, for example we have `class > SomeAdmin(TRANSLATED and TranslationAdmin or admin.ModelAdmin)` all through > each of the admin modules. I think we could have a single BaseAdmin class > with this logic that everything subclasses, rather than having conditions > all throughout the code base. With that in place I figure it'd be pretty > easy for us to modify that base class to include a snippet of jQuery with a > single toggle (eg per screen, not per field, as Ed suggested) that > shows/hides all the language fields. > > Also confused about the end of mezzanine/conf/__init__: > > TRANSLATED = settings.USE_MODELTRANSLATION and settings.USE_I18N > CODE_LIST = [lang[0] for lang in settings.LANGUAGES] > > This isn't where settings should simply be dumped. CODE_LIST could > possibly be a function in mezzanine.utils.translation. Does Django not have > this already? > > More importantly I don't understand the relationship between > USE_MODELTRANSLATION and USE_I18N, and why these get combined into another > setting (which isn't consistently used throughout the PR, in same cases > this combo of settings get checked separately again). Can't everything just > check the USE_MODELTRANSLATION settings? If USE_I18N is needed for this to > work, then that's what's set_dynamic_settings is for - fixing up missing > things that the developer has intended to have working by providing a > single point of configuration that hides away all the various bits that > need configuring. In set_dynamic_settings we should simply check for > USE_MODELTRANSLATION and if True, add modeltranslation app if missing, and > if USE_I18N actually needs to be True for modeltranslation to work, set it > to True as well. modeltranslation shouldn't be in INSTALLED_APPS by default > either. > > Lastly we probably don't want the language chooser for the front-end site > in the top nav, there's really no room there. Perhaps it could sit on the > right-hand panel at the top somewhere along with the login bits, > conditionally, if USE_MODELTRANSLATION is True. All of this stuff should be > turned off by default which means USE_MODELTRANSLATION defaults to False > and *everything* else stems from that. > > If I've stupidly overlooked anything please let me know. This looks real > promising and I'm hoping this along with django-rest-framework integration > will make a compelling offering for the next major release 3.2. I'm going > to push 3.1.5 shortly with some bug fixes, and hopefully that'll be the > last 3.1.x release. > > Thanks Mathias and Ed for working this all out, it looks really really > good. > > > > > > > > On Sat, Jun 7, 2014 at 2:31 PM, Stephen McDonald <[email protected] > <javascript:>> wrote: > >> Real odd, works fine for me now and no matter what I try I can't >> reproduce the error I had. >> >> >> On Sat, Jun 7, 2014 at 2:09 PM, Eduardo Rivas <[email protected] >> <javascript:>> wrote: >> >>> Odd, I just created a project without issue. This is my venv: >>> >>> Django==1.6.5 >>> Mezzanine==3.1.3 #From Mathias' branch >>> Pillow==2.4.0 >>> South==0.8.4 >>> argparse==1.2.1 >>> bleach==1.4 >>> django-debug-toolbar==1.2.1 >>> django-modeltranslation==0.7.3 >>> filebrowser-safe==0.3.3 >>> future==0.9.0 >>> grappelli-safe==0.3.10 >>> html5lib==0.999 >>> oauthlib==0.6.1 >>> psycopg2==2.5.3 >>> pytz==2014.4 >>> requests==2.3.0 >>> requests-oauthlib==0.4.0 >>> six==1.6.1 >>> sqlparse==0.1.11 >>> tzlocal==1.0 >>> wsgiref==0.1.2 >>> >>> I ran mezzanine-project and createdb --noinput. Have you done any other >>> changes to settings? >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Mezzanine Users" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected] <javascript:>. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> >> >> -- >> Stephen McDonald >> http://jupo.org >> > > > > -- > Stephen McDonald > http://jupo.org > -- You received this message because you are subscribed to the Google Groups "Mezzanine Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
