New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using sane defaults #200

Closed
justintadlock opened this Issue Oct 20, 2014 · 1 comment

Comments

Projects
None yet
2 participants
@justintadlock

justintadlock commented Oct 20, 2014

I'm bringing this to the attention of the WordPress.org theme review team, so I wanted to give you a heads up and get everyone on the same page. I'm not overly familiar with the Options Framework, so if I get anything incorrect, please let me know.

Many theme authors use the Options Framework for their theme options on WordPress.org. As of WP version 3.9, we've had a requirement that themes must use sane defaults. This means that they cannot write any options to the database by default. The only way a theme should write options to the database is by the user explicitly saving the options. Otherwise, a default should be used.

Since the Options Framework uses get_option(), it should use the second parameter of $default for default options, for example.

The particular code in question is the Options_Framework::set_theme_option() method, which is hooked to admin_init. From what I can tell, this method writes a default option to the database. https://github.com/devinsays/options-framework-plugin/blob/master/includes/class-options-framework.php#L37

I'm not sure if there are other areas in the framework that write to the database by default. This is just the one thing I noticed. I'll be more than happy to work with you to get things resolved.

@devinsays

This comment has been minimized.

Owner

devinsays commented Oct 20, 2014

I believe it does set an option to identify the correct option name for the theme and also to delete data if the plugin is removed. Perhaps the value can just be returned rather than set as an option. Let me do some testing on it. Thanks for opening the ticket and bringing it to my attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment