[theme-reviewers] Theme quality review process

[Darren Slatten]

> Anyway, that sort of question is kinda petty when you get right down
> to it. Theme review is an ongoing process, and the guidelines will
> change as needed. For now, I agree with the no-PHP-notices rule, as it
> shows an attention to detail. I myself don't always check my plugins
> to not produce notices, but it's something I know that I should do.
>

I completely agree. However, the theme I'm working on right now throws 100+
notices in some situations. They are all due to undefined variables/indices
that are used to store the theme's options. Unfortunately, the options panel
only saves the option values that have been explicitly set (so for example,
an option with an empty checkbox is not saved as false--it's simply left
undefined). Then...throughout the entire theme, the author used a code
pattern like this:

if ( $themeslug_options['footer']['show_copyright_info'] )
    themeslug_copyright();

Basically, the author is working with the assumption that *undefined* will
behave just like *false* (and honestly, I can't think of a relevant scenario
where this assumption would cause problems).

So in a case like this (keeping in mind that this theme has a huge amount of
options), flushing out all the Undefined notices would require substantial
changes to the code. What would be the best way to resolve this? The
solutions I see are:



Change all occurrences of this:

if ( $foo['bar'] )

...to this:

if ( isset( $foo['bar'] ) && $foo['bar'] )



OR



Rewrite the options panel (~3,000 lines of code).



I'm guessing the latter solution is preferable?

-Darren




On Wed, Jun 8, 2011 at 1:56 PM, Otto <otto at ottodestruct.com> wrote:

> On Wed, Jun 8, 2011 at 1:49 PM, Darren Slatten <darrenslatten at gmail.com>
> wrote:
> > I could be wrong, but I got the feeling that part of the question was
> along
> > the lines of:
> >
> > If PHP notices are not permitted, then why do some already-accepted
> themes
> > generate PHP notices?
>
> Obviously, no theme review process involving human judgment is going
> to be perfect.
>
> In the end, you're just going to have to say "just because theme X has
> the problem doesn't make it not actually a problem".
>
> Anyway, that sort of question is kinda petty when you get right down
> to it. Theme review is an ongoing process, and the guidelines will
> change as needed. For now, I agree with the no-PHP-notices rule, as it
> shows an attention to detail. I myself don't always check my plugins
> to not produce notices, but it's something I know that I should do.
>
> -Otto
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110608/b9d7bb89/attachment.htm>