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

Reply via email to