[theme-reviewers] Would like to review a theme please.

michael at mfields.org michael at mfields.org
Mon Mar 28 14:10:15 UTC 2011


Cais,

Thanks for taking the time to answer my questions! I'm still a bit foggy
on #3 though. Are headings within post_content intended to be clearing
blocks? I know that Twentyten does this, but honestly, this was the first
time I had ever seen it done before. It's a good idea IMO and I do it
myself now. Is it expected that all themes do this too?

Best,
-Mike

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




More information about the theme-reviewers mailing list