jrief / djangocms-cascade

Build Single Page Applications using the Django-CMS plugin system
MIT License
165 stars 85 forks source link

[docs] Suggest lists instead of tuples in settings #214

Closed bittner closed 7 years ago

bittner commented 7 years ago

The documentation suggests using tuples for most of the settings, e.g. Django's INSTALLED_APPS, Cascade plugins, leaf plugins.

Now even Django core has switched to the (I think, semantically correct) use of lists in Python for everything that is of the same type (I believe), e.g. INSTALLED_APPS. Could the Cascade docs also suggest using lists instead of tuples where appropriate? Technically, it works in the code, as for what I have tried.

I'd prepare a PR for the docs, if that's alright.

jrief commented 7 years ago

Thanks for the hint. After looking for references to this claim, I came across this post on Stackoverflow which states that once the documentation said: "For settings that are sequences, use tuples instead of lists. This is purely for performance."

This doesn't seem to be true anymore. Therefore in one of my next versions I'll have to refactor this.

bittner commented 7 years ago

Another thought:

What about refactoring CMSPLUGIN_CASCADE_PLUGINS into CMSPLUGIN_CASCADE['plugins']? Would that be an option? This way we'd only have to define a single property for Cascade in Django's settings, e.g.

CMSPLUGIN_CASCADE = {
    'plugins': ['cmsplugin_cascade.bootstrap3'],
    'alien_plugins': ['TextPlugin'],
}

Would be a nice side-by-side of regular Cascade plugins and their alien friends.

jrief commented 7 years ago

Yes, another issue on my TODO-list. Thanks for reminding me.

jrief commented 7 years ago

Settings in the examples have been changed now, hence closing.

bittner commented 7 years ago

There are still occurrences of tuples where lists could be used, e.g.