[2013-03-12] Consistent usage of Config API instead of statics in core

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS

https://www.silverstripe.org

BSD 3-Clause "New" or "Revised" License

721 stars 821 forks source link

[2013-03-12] Consistent usage of Config API instead of statics in core #1704

Closed silverstripe-issues closed 11 years ago

silverstripe-issues commented 11 years ago

created by: @chillu (ischommer) assigned to: @chillu (ischommer) created at: 2013-03-12 original ticket: http://open.silverstripe.org/ticket/8317

See https://github.com/silverstripe/sapphire/pull/1244

Change all static access of public statics to using the Config API
Set those statics to private (only way to throw errors)
Deprecate static setters and getters
Fix documentation, e.g. usage of Director::addRules() or Object::add_extension()

silverstripe-issues commented 11 years ago

comment by: @chillu (ischommer) created at: 2013-03-20

Work in progress: https://github.com/chillu/sapphire/tree/experiments/config-statics

silverstripe-issues commented 11 years ago

comment by: @chillu (ischommer) created at: 2013-03-21

TODO

[x] CMSMenu::$menu_items
[x] Director::addRules()-
[x] i18n::get_date_format(), i18n::get_time_format()
[ ] SS_Cache::cache_lifetime
[x] Permission::declared_permissions

Note: Moved class-based setings into #1678 instead

silverstripe-issues commented 11 years ago

comment by: @tractorcow (tractorcow) created at: 2013-03-25

Can you give me a run down on why statics now must be all private? I believe (if I understand things correctly) that statics are deprecated for configuration. You mention that this is to throw an error. Is this an error that's thrown when the API is used incorrectly?

Thanks in advance for clarifying.

silverstripe-issues commented 11 years ago

comment by: @tractorcow (tractorcow) created at: 2013-03-25

Also,

Fatal error: Access level to ErrorPage::$defaults must be public (as in class Page) in \cms\code\model\ErrorPage.php on line 292

This is what prompted me to ask this, thanks!

silverstripe-issues commented 11 years ago

comment by: @tractorcow (tractorcow) created at: 2013-03-25

I think, after actually reading all the related pull request / comments I understand after all. I'll just fix my obsolete code and retreat here. :)

silverstripe-issues commented 11 years ago

comment by: @tractorcow (tractorcow) created at: 2013-03-25

Just in any case someone else has the same issues I have, the only thing one must do to access any of these configuration variables is to replace this code:

$allowedImageExtensions = File::$app_categories['image'];

With code that looks like the below:

$allowedImageExtensions = File::config()->app_categories['image'];

I now see the value in privatisation here, as it forces me to use the correct api, rather than the lazy way. :)

silverstripe-issues commented 11 years ago

comment by: @chillu (ischommer) created at: 2013-03-25

Its all detailed in http://doc.silverstripe.org/framework/en/3.1/changelogs/3.1.0#static-configuration-properties-are-now-immutable-you-must-use-config-api BTW. Let me know if those docs are comprehensible, its going to be an important part of the upgrade experience.

wilr commented 11 years ago

Grepped public variables and looks like most of the variables have been updated and deprecations have been added.