Hey Mike -<br><br>re: Questions:<br><br>1. I'm not really having issues with this considering you stated it is being escaped correctly.<br><br>2. PHP notices are generally enough to set to "not-approved" as we expect there to be none in the theme.<br>
<br>3. Clearing tags appropriately is also generally considered for "not-approved" but not usually if it is a single issue and no others are found.<br><br>4. Given that not passing the Theme Check will actually not allow a theme to be uploaded now you could stop there; but, since the theme's currently in the queue were not subjected to that criteria they can be further reviewed. Of course, if you considered the Theme Check review to be epically failed then there really is not much sense as I see it in continuing.<br>
<br><br>The Theme Review comments appear well written and informative ... without actually reviewing the theme myself I would say you have given it a proper review.<br><br><br>Cais.<br><br><div class="gmail_quote">On Sat, Mar 26, 2011 at 10:48 PM, <span dir="ltr"><<a href="mailto:michael@mfields.org">michael@mfields.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Thanks Chip!<br>
<br>
Gave the first theme a look over and I wanted to get some feedback from<br>
everyone about my observations. This is the first theme that I have<br>
reviewed so I am not sure about a few things.<br>
<br>
It appears that this theme has undergone a cursory review by Chip Bennett<br>
<a href="http://themes.trac.wordpress.org/ticket/3015" target="_blank">http://themes.trac.wordpress.org/ticket/3015</a> In this scenario, I'm<br>
guessing it would be up to me to test all other areas not covered in the<br>
previous review. I've checked quite a few things at this stage and<br>
personally I would fail the theme. I've listed the following reasons why<br>
below. Please let me know your thoughts on what is causatioon for a failed<br>
review - I'm still a bit confused here. I also have a few questions if<br>
someone has the time.<br>
<br>
Thanks!<br>
-Mike<br>
<br>
Questions:<br>
<br>
1. The theme provides a textarea in a custom settings page that allows the<br>
user to display javascript in the frontend. My tests so far have shown<br>
that care has been taking so that the code is correctly escaped on the<br>
backend however the code is executed in the template files. Is this<br>
allowed?<br>
<br>
2. Are php notices fail material?<br>
<br>
3. What about clearing h2 tags in the post content? This theme does not do<br>
this and it makes for messy (IMHO) display of the floated image tests. Is<br>
this something that I should report as a suggestion? Is it grounds for<br>
failure?<br>
<br>
4. Did I go far enough on the review? Too far? I would have failed the<br>
theme for it not passing theme check requirements. Should I have just<br>
stopped here?<br>
<br>
<br>
==================<br>
REVIEW STARTS HERE<br>
==================<br>
<br>
Theme Check Plugin - Required<br>
-----------------------------<br>
The theme doesn't have post pagination code in it. Use posts_nav_link() or<br>
paginate_links() or next_posts_link() and previous_posts_link() to add<br>
post pagination.<br>
<br>
<br>
Theme Check Plugin - Recommended<br>
--------------------------------<br>
1. Text domain problems in search.php. You have not included a text<br>
domain! Line 12: $count = $allsearch->post_count; _e('');<br>
2. Theme URI: is missing from your style.css header.<br>
<br>
<br>
Theme Unit Test: Many Categories<br>
--------------------------------<br>
No categories are displayed in single post view. Line 27 of index.php has<br>
what I believe to be a logic error where get_the_tag_list() is checked<br>
twice: if ( get_the_tag_list() || get_the_tag_list() ). I would suggest<br>
checking for categories as well here.<br>
<br>
<br>
Theme Unit Test: Post Format Test: Image (Linked)<br>
-------------------------------------------------<br>
The image in this post breaks out of the content area and spills into the<br>
sidebar.<br>
<br>
<br>
Attachment Views<br>
----------------<br>
Clicking on one of the images from the "Post Format Test: Gallery" post<br>
will bring you to the view that I am talking about here. There are no<br>
links to navigate to the previous or next image in the gallery. There are<br>
also two php notices generated in this template:<br>
<br>
Notice: Undefined variable: cmpvalues in core\plugins\seo-features.php on<br>
line 409<br>
Notice: Undefined offset: 0 in<br>
core\plugins\breadcrumbs\yoast-breadcrumbs.php on line 211<br>
<br>
I would suggest that you create an attachment.php template that will<br>
handle the display of all attachments. The following link shows the<br>
template heirarchy that is used for these views: <a<br>
href="<a href="http://codex.wordpress.org/Template_Hierarchy#Attachment_display" target="_blank">http://codex.wordpress.org/Template_Hierarchy#Attachment_display</a>">Attachments</a>.<br>
<br>
<br>
Empty Result Set - php notices<br>
------------------------------<br>
Please enter the a string that will produce no results ("123" worked for<br>
me) into the search form located in the upper right-hand side of the<br>
theme. 5 php notices are generated when there is no result set to display.<br>
They are all created by the following file on lines 404, 406 and 409.<br>
\3.1\wp-content\themes\wtrt-billions\core\plugins\seo-features.php. These<br>
notices are not limited to search results. They extend into any case where<br>
an empty result set is returned including: non-existant categories and<br>
tags.<br>
<br>
<br>
Search Results (page two only) - php notices<br>
--------------------------------------------<br>
In the event that a search query produces more than one page of results,<br>
page 2 always seems to display the following notice.<br>
"Notice: Undefined variable: cmpvalues in<br>
C:\xampplite\htdocs\3.1\wp-content\themes\wtrt-billions\core\plugins\seo-features.php<br>
on line 409". This appears to be the same notice as reported above. Fixing<br>
one *should* fix both.<br>
<br>
<br>
Paged Navigation in Blog View<br>
-----------------------------<br>
There is no paging for the blog view. This appears to be true where the<br>
front page is set to display "Blog posts" as well as when a page is<br>
assigned as the "Posts page". I see that there is a custom page template<br>
(blog-template.php) which I beilive has been created to remedy this<br>
situation, but it will only ever fix one of these scenarios. I would<br>
suggest creating a new template (home.php) which WordPress will use in<br>
both cases listed above. Please see the following codex article for<br>
clarification on this:<br>
<a href="http://codex.wordpress.org/Template_Hierarchy#Home_Page_display" target="_blank">http://codex.wordpress.org/Template_Hierarchy#Home_Page_display</a><br>
<div><div></div><div class="h5"><br>
<br>
> Here you go!<br>
> <a href="http://themes.trac.wordpress.org/ticket/3278" target="_blank">http://themes.trac.wordpress.org/ticket/3278</a><br>
> <<a href="http://themes.trac.wordpress.org/ticket/3278" target="_blank">http://themes.trac.wordpress.org/ticket/3278</a>><br>
> <a href="http://themes.trac.wordpress.org/ticket/3215" target="_blank">http://themes.trac.wordpress.org/ticket/3215</a><br>
> <<a href="http://themes.trac.wordpress.org/ticket/3215" target="_blank">http://themes.trac.wordpress.org/ticket/3215</a>><br>
> <a href="http://themes.trac.wordpress.org/ticket/3294" target="_blank">http://themes.trac.wordpress.org/ticket/3294</a><br>
><br>
> <<a href="http://themes.trac.wordpress.org/ticket/3294" target="_blank">http://themes.trac.wordpress.org/ticket/3294</a>>There are 3 to start with.<br>
> Have fun!<br>
><br>
> Chip<br>
><br>
> On Sat, Mar 26, 2011 at 7:20 PM, <<a href="mailto:michael@mfields.org">michael@mfields.org</a>> wrote:<br>
><br>
>> Opps, forgot about that part its mfields all lowercase.<br>
>><br>
>> Thanks!<br>
>><br>
>> > Hey Mike, what's your WPORG username (case-sensitive)?<br>
>> ><br>
>> > Chip<br>
>> ><br>
>> > On Sat, Mar 26, 2011 at 6:57 PM, <<a href="mailto:michael@mfields.org">michael@mfields.org</a>> wrote:<br>
>> ><br>
>> >> Please hook me up with a theme or two for review. Can I keep you guys<br>
>> >> posted of any questions I have along the way? I'll try to condense<br>
>> them<br>
>> >> into as few emails as possible.<br>
>> >><br>
>> >> Thanks!<br>
>> >><br>
>> >> -Mike<br>
>> >><br>
>> >> _______________________________________________<br>
>> >> theme-reviewers mailing list<br>
>> >> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> >> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> >><br>
>> > _______________________________________________<br>
>> > theme-reviewers mailing list<br>
>> > <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> > <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> ><br>
>><br>
>><br>
>> _______________________________________________<br>
>> theme-reviewers mailing list<br>
>> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>><br>
> _______________________________________________<br>
> theme-reviewers mailing list<br>
> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
><br>
<br>
<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
</div></div></blockquote></div><br>