This is a child of #1775842: [meta] Convert all variables to state and/or config systems
The following calls to variable_get are made in API documentation. These should be removed or replaced with the Drupal 8 equivalent.
core/modules/node/node.api.php
375 $restricted = variable_get('example_restricted_roles', array());
core/modules/system/system.api.php
943 '#default_value' => variable_get('upload_' . $form['type']['#value'], 1),
1932 if (strpos(file_uri_target($uri), variable_get('user_picture_path', 'pictures') . '/picture-') === 0) {
2593 * variable_get() if you are collecting any data that you need to store and
2655 $myprofile_needs_batch_processing = variable_get('myprofile_needs_batch_processing', FALSE);
All calls to variable_set should also be replaced.
Comment | File | Size | Author |
---|---|---|---|
#8 | 2109065-update-api-docs-8.patch | 13.17 KB | jhodgdon |
#6 | 2109065-update-api-docs-6.patch | 8.71 KB | ianthomas_uk |
#1 | 2109065-update-api-docs-1.patch | 6.53 KB | ianthomas_uk |
Comments
Comment #1
ianthomas_ukHere are my changes. This doesn't fix the user_picture_path variable, as that looks a bit more complicated (I'll probably open a new issue for it).
Comment #2
ianthomas_ukI removed this variable completely as it is not referred to anywhere else and the if statement didn't seem to serve a useful purpose (with this sort of code, if you didn't want it to run you'd probably just delete the code from your custom module or uninstall the contrib module).
Comment #3
ianthomas_ukTernary operators should be removed
Comment #4
alexpottLets change this to state() rather then config because the whole namespacing system in CMI is designed to avoid this :)
This is very tricky. Storing something in CMI means that you want to be able to deploy the configuration. Typically that is not information that is collected during installation. For example, the profile used to be stored the variable system but is now written to settings.php which is actually very sensible considering what modules are available to enable depend on this. I think at least it is worth mentioning that config() should only be used to store values that are changeable by the user after installation. If this not the case then it's not config.
You are no longer cleaning up a variable :)
Comment #5
ianthomas_ukRE #4.2
I mentioned config to try and avoid people thinking they couldn't use config, so putting things in state, only to put them into config later on. I'll try and clarify that.
Comment #6
ianthomas_ukHere's a new patch with quite a few changes.
- No ternary operators used to provide config defaults
- State keys now start 'mymodule.' or 'myprofile.'
- State defaults provided via the second parameter of get()
- lousy_module variable renamed and moved to state
- hook_modules_uninstalled rewritten to mirror hook_modules_installed
- hook_install/hook_uninstall replaced with example from image.module, as this seemed to be a good demonstration of setting and unsetting something that is not managed by Drupal itself
- Installation guidance rewritten, removing reference to config.
TODO
- I'm not sure how to avoid the dynamic config on line 943 of system.api.php
Comment #7
alexpottI'm trying to remember why / if a dynamic config key name like this is an issue. Perhaps a better way of doing this like this...
Nice! Although this appears out of scope actually this is making the documentation consistent.
Comment #8
jhodgdonHere's a new patch.
I was unable to make an interdiff (technical difficulties). sorry. Things I changed:
- Used Alex's suggestion (more or less) from #7.
- Found a few more places where variable_* was being used in API comments, and removed them too.
Comment #9
jhodgdonAs a note:
After this patch... I grepped and the only place with variable_* in a core/modules/*.api.php file is in menu.api.php; this is covered by a separate issue:
#2108679: API documentation: Convert my_module_menus examples in menu.api.php to CMI
I also grepped through the whole codebase for variable_* in a line containing a * (i.e., in a doc block) and this patch got all the varable_*() calls.
Comment #11
jhodgdon#10 looks like a bot glitch. The tests passed. ?!?
Comment #12
mtiftYes, the tests passed. Someone re-ran the tests on the backed.
Comment #13
cosmicdreams CreditAttribution: cosmicdreams commentedComment #14
alexpottCommitted a6efca1 and pushed to 8.x. Thanks!
Comment #14.0
alexpottUpdated issue summary.