WordPress.org

Ready to get started?Download WordPress

Theme Review Team

Recent Updates Page 3 Toggle Comment Threads | Keyboard Shortcuts

  • Tammie 11:43 am on January 21, 2015 Permalink |  

    Explaining and giving examples to requirements

    We’ve got a new handbook page now to start collection some examples and further explanations for required items. It’s been brought up a lot that some could do with having extra information. So, now we have a page right here: https://make.wordpress.org/themes/handbook/review/required/explanations-and-examples/.

    This may be a temporary page as we can look to bring in things to the main required page. If we keep this, we do need to link between the two documents. A decision on having these two or merging hasn’t been made yet, this is just about starting to expand and be clearer.

    Next week’s meeting will focus on adding to this section. Please bring ideas and content for things that can be brought into this section and really help make our requirements clear.

    A little note, we don’t want to call people out in our examples. We all have to learn. If you are going to use a code example use one that doesn’t tie to a theme on the repo. Examples from the theme developer handbook or https://developer.wordpress.org/ are ok to use and link back to.

     
  • Tammie 6:52 pm on January 20, 2015 Permalink |  

    Theme review team weekly notes

    The logs are here:
    https://wordpress.slack.com/archives/themereview/p1421776726000809

    We are not going to be putting the handbook on Github. We are going to use the workflows already in place and used by other teams. However, so we can keep the handbook fresh and check in with it, we are going to cover a new section each week for a few months. This will also enable in the weekly meeting people to get more familiar and discuss it.

    We will also have a page up for suggestions. People can then add their thoughts to that for changes and updates. This should help us collate ideas.
    https://make.wordpress.org/themes/handbook-suggestions/

    From now on, at the end of each weekly meeting we will also give time every week for change requests and updates. This gives us all a chance to highlight issues, areas of weakness and areas that can be improved. Bring your best ideas and suggestions.

     
    • Emil Uzelac 2:20 am on January 21, 2015 Permalink | Log in to Reply

      Thanks Tammie :)

      I am glad that we are not going with the GitHub for the handbook.

      Great meeting, short and to the point!

    • Ulrich 12:32 am on January 25, 2015 Permalink | Log in to Reply

      We should talk about this in the next meeting https://github.com/Otto42/theme-check/pull/42

    • Justin Tadlock 6:15 pm on January 25, 2015 Permalink | Log in to Reply

      Next meeting item of discussion: Custom CSS theme options.

      This has come up in a few recent tickets. Themes are allowing end users to enter arbitrary custom CSS via a textarea in their theme options. It might be time to consider putting this under the “plugin territory” category. There are 3 major issues I see:

      • Handling permissions (using the correct capability).
      • Sanitizing before saving to database.
      • Handling the output.

      There may be cases where this makes sense in a theme, but for general purposes, I’d say we need to leave this to plugin authors with the skillset to make this safe and work correctly. It should be up for discussion anyway.

      Some relevant links:

      • usability.idealist 8:41 pm on January 26, 2015 Permalink | Log in to Reply

        A note on this: These are regular issues with ANY kind of option, most of which are already dealt with (using the Settings API).

        So the actual ONLY issue is a missing CSS sanitizer (the only existing one is used for INLINE css, which might be found inside tags of regular posts, but certainly not stand-alone fields).

        I’ve been dealing with this the last few days. One option is using the current version of CSSTidy, which might be a bit of overkill, but then, it’s only being used inside the admin section of your theme, so it shouldn’t too much harm.

        Example approach: http://wordpress.stackexchange.com/questions/53970/sanitize-user-entered-css/165496#165496
        Current (!) version of CSSTidy: https://github.com/Cerdic/CSSTidy

        The second one is my current approach: Stripping out anything that is NOT CSS. So, nasty stuff like dropping JS code into it, etc. is already being avoided.

        Third option would be writing your very own, which I tried, but abadoned pretty soon.

        cu, w0lf.

        • Justin Tadlock 10:11 pm on January 26, 2015 Permalink | Log in to Reply

          Permissions, sanitizing, and escaping are definitely issues with any kind of option. However, CSS blocks present their own issues, as I’m sure you’re aware. Using CSSTidy or a custom script might be a good solution, but this is code that one of the reviewers on the review team will have to review for any theme using it. Not many have the necessary skills to review such code.

          There’s also the issue of whether such a setting belongs in a theme at all. I’m sure we’ll cover these points tomorrow in the meeting.

    • smartcat 4:28 pm on January 27, 2015 Permalink | Log in to Reply

      Is there a process outlined in theme review regarding how long a reviewer has to review a theme ? reason I ask is because I have a theme that was assigned to a reviewer who has been idle for a month and I fear that it will just sit there permanently https://themes.trac.wordpress.org/ticket/21699

    • Tammie 4:29 pm on January 27, 2015 Permalink | Log in to Reply

      @smartcat, they should respond within 7 days. We triage all tickets each week as best we can by hand. You can always bring up a ticket in Slack at #themereview. That’s a better place than on our update lists if you want to get eyeballs on it.

  • Tammie 8:51 pm on January 19, 2015 Permalink |  

    This week we’ve got a chat again at 18:00 UTC on Tuesday in #themereview. There is no set topic, so add a comment or bring your topics to the chat.

     
  • Justin Tadlock 8:26 pm on January 15, 2015 Permalink |  

    Let’s be more specific in our reviews 

    Hi, all. I’ve been away for a couple of weeks because of sickness and some personal life stuff. I wanted to catch up with things, so I started looking through our review queues.

    One of the troubling things I noticed is that we’re not specific enough in our advice. This doesn’t go for all reviewers. Some of you are doing a great job, but it never hurts to look at your reviews to make sure you’re helping theme authors the best you can. What I see happening in a few reviews is copying/pasting the guidelines. That’s an awesome thing, but if you don’t put it into context for the theme author, they might not know where to look to fix the issue.

    Here’s an example quote I pulled from a couple of reviews:

    Validate and sanitize untrusted data before entering into the database. All untrusted data should be escaped before output.

    Yes, the theme author needs to follow that guideline, so it’s good to let them know about it. However, it doesn’t point out the actual issue with their code. We need to add something more helpful. For example:

    On line 333 of /inc/theme-options.php, the $setting['front_posts_number'] option isn’t sanitized before it’s saved to the database. absint() would be a good fit.

    Notice how one is specific and gives actionable advice while the other doesn’t? Our reviews should lean toward the specific as much as possible.

    Sometimes, there may be a number of issues that are the same, so being too specific would take too much time. Using the above example, let’s assume the theme author has multiple sanitizing issues to correct. Here’s how I’d write it:

    In the /inc/theme-options.php file, you have multiple options that need to be sanitized before saving to the database. If you need more specific help with this, don’t hesitate to ask me.

     
    • usability.idealist 8:48 pm on January 15, 2015 Permalink | Log in to Reply

      That’s also what otto has been suggesting for the Theme Check plugin – more specific messages.

      Eg. the “missing sanitize callback” check is nice, but it doesnt tell you exactly WHERE the bugger is. When pondering throught roughly 150 customizer options, as one theme, which I partially developed, is using, a simple “oh, there is ONE callback missing .. somewhere” is just like those Windoze messages asking if you REALLY REALLY want to do this or that – ie. pretty much useless.

      I’d be quite happy to see a more issue-focused approach to this ;)

      cu, w0lf.

    • MRWweb 9:13 pm on January 15, 2015 Permalink | Log in to Reply

      I would just like to heartily endorse Justin’s point. As someone who just went through a review, the feedback I was getting was frustratingly vague sometimes and even lead to me think the opposite of what was said at one point. Worse, I ended up having my theme approved before I had correctly dealt with the issues (I resubmitted thinking that I had).

      • Emil Uzelac 10:08 pm on January 15, 2015 Permalink | Log in to Reply

        And hopefully I took care of your final issues properly :)

        • MRWweb 5:10 pm on January 16, 2015 Permalink | Log in to Reply

          Yeah. everything worked out just fine, but I did get a little frustrated since it felt like I was going in circles there for a while. From my experience, I can also say that some Child Theme-specific guidelines (textdomain in particular) probably would have helped me avoid most of the issues in the first place.

    • Carolina Nymark 10:24 pm on January 15, 2015 Permalink | Log in to Reply

      MRWeb yes that wasn’t my best review. There are a lot of things that we need to know and sometimes we ( I ) give the wrong advice. We dont have any other resourses to check the theme against than you have.
      Thankfully there is always two people looking at a ticket.

      Sometimes though its really hard to know what level to write at, especially when the author doesn’t get back to you. If a theme has a few errors and overall quality is high, its is easy to be vague because you assume the author will understand.

      There are also themes where you feel like you are rewriting the entire theme for them and that’s not the purpose and it is very time consuming.

      • Emil Uzelac 10:41 pm on January 15, 2015 Permalink | Log in to Reply

        We all try our best and it happens. And like you said, there is always at least couple of us involved.

      • MRWweb 5:11 pm on January 16, 2015 Permalink | Log in to Reply

        No worries. Every experience is a learning experience. I certainly learned some stuff too and that’s good! I’m glad the theme is up now :)

        Justin’s suggestion above to add specific code references probably would have avoided a couple of those back and forths.

    • Carolina Nymark 10:32 pm on January 15, 2015 Permalink | Log in to Reply

      I suppose authors would be more inclined to communicate with the reviewer if there wasn’t a 7 week wait. I would rather discuss the review and try to answer questions than spend hours looking through code that the author no longer has any interest in.

      • usability.idealist 8:16 pm on January 17, 2015 Permalink | Log in to Reply

        Maybe “pushing” theme authors more towards the Slack channel would be more of help?
        I got all my issues sorted out BEFORE even adding the theme for review, just by pestering folks on Slack ;)

        And of corpse, it might be good to point to those conversations, for quicker solutions or kind of a knowledege base. A la “question: Has there everywhere to be a sanitze_callback for the Theme Customizer” – “answer: yea, it has to be. for a detailed example see [a href=”url in the slack channel archive”]the conversation of this-and-that-date[/a]” or similar ;)

        cu, w0lf.

        ps: On a sidenote: Yeah, it’s been just an “update”, but still .. quite the big one.

    • hiddenpearls 7:29 am on January 19, 2015 Permalink | Log in to Reply

      awesome advise @Justin

    • Jose Castaneda 12:01 am on January 20, 2015 Permalink | Log in to Reply

      :thumbsup: Great write up!

  • Tammie 6:57 pm on January 13, 2015 Permalink |  

    Theme review team weekly notes

    The logs are here:
    https://wordpress.slack.com/archives/themereview/p1421171962000019

    The floor was open some of the topics were:

    • Issues with mentoring workflows and consistency.
    • Clarification and extra supporting information needed for requirements in Handbook.
    • We are going to collect topics to discuss and bring back the weekly themes for chats. If you’ve got something you’d like covered just mention it here in the comments.
     
  • Tammie 3:14 pm on January 12, 2015 Permalink |  

    Weekly chat agenda

    We’ll have two chats this week, first up will be Tuesday’s 18:00 UTC meeting in Slack #themereview. We will then also have the Thursday 18:00 Mentoring meeting.

    There is no agenda for any of the meetings. If you’ve got something you’d like to add please note it here. We will also have an open office so that anyone can ask whatever they want.

    I would like to start having subjects again for our meetings, so perhaps we can also talk about that. We had this for a bit last year and it worked really well moving things on.

     
  • Tammie 6:36 pm on January 6, 2015 Permalink |  

    Theme review team weekly notes

    The logs are here:
    https://wordpress.slack.com/archives/themereview/p1420567275004160

    Notes:

    • https://core.trac.wordpress.org/ticket/30770 needs to make sure we are all on the same page and help with it.
    • We talked about removing the trainee reviewer role. If nobody objects this would be good to do. Please add a comment if an issue with this.
    • Lets try and put the handbook on Github as an experiment with a clear workflow outlined. We’ll review this in a month.
    • We have a handful of people mentoring. They are awesome, but they are also awesome with a lot of people mentoring. Simply put, we need more people to scale this mentoring program. Now if you’re not ready, that’s totally fine. But, if you are a longer standing reviewer (+6 months), maybe considering mentoring would be awesome.

    Here’s to a great new year reviewing!

     
  • Tammie 1:03 pm on January 5, 2015 Permalink |  

    Weekly chats starting back up.

    I hope everyone had a great holidays and is all recharged ready to get reviewing in 2015!

    This week the weekly chats will start back up. First up will be Tuesday’s 18:00 UTC meeting in Slack #themereview. We will then be also bringing back the Thursday 18:00 Mentoring meeting.

    I have a few things that have come up the past few weeks we could do with discussing in this week’s meeting:

    • It would be great to get a team consensus on this ticket: https://core.trac.wordpress.org/ticket/30770 .
    • Should we remove trainee reviewer role: https://meta.trac.wordpress.org/ticket/752?
    • What of the new template tags in 4.1 should we look to require? https://make.wordpress.org/core/2014/12/04/new-template-tags-in-4-1/
    • Lets get a date for the options and customizer learnup. @greenshady can you update on this?
    • We spoke about doing one weekly meeting every now and then via Hangouts to see everyone’s face and hear voice. What about doing next week?
    • @greenshady you were going to write a post on custom CSS. How is that coming on?
    • @jcastaneda how is doing_it_wrong doing?
    • Putting the handbook on Github. It was mentioned by @Ulrich about this. I’m happy to get this happening and do it next week myself. There are a few things we’d need to do and have understood for this to work, lets go over those in the meeting. I think it’s a good way to also discuss requirements in a place and get more people able to be hands on.
    • Mentors, we need more are you up for the challenge?
     
    • Justin Tadlock 4:38 pm on January 5, 2015 Permalink | Log in to Reply

      What of the new template tags in 4.1 should we look to require? https://make.wordpress.org/core/2014/12/04/new-template-tags-in-4-1/

      I’m just going to throw out a few notes on the template tags:

      For archive titles/descriptions, we should only require their use if the theme shows an archive title/desc. Not all of them do nor should they. We might want to also recommend until, say, version 4.3 or so since they’re new template tags.

      For navigation, those functions should be added to the list of allowed nav functions.

      For the title tags, that’s an optional theme support feature. We should probably avoid making any optional theme features required. Recommend.

      Lets get a date for the options and customizer learnup. @greenshady can you update on this?

      I’ll start work this week on this. I’m just now getting back into the swing of things. I’m thinking we can do it in two weeks.

      @greenshady you were going to write a post on custom CSS. How is that coming on?

      I had thought about writing something like that on my blog. It’d be a long and complex tutorial that wouldn’t be easy to read with the P2 blog design we have here.

  • Emil Uzelac 5:22 pm on December 26, 2014 Permalink |  

    Preliminary Theme Review Process 

    Note: This post is not an official guideline, best practice or advice.


     

    Licensing:

    Always pay extra attention and make sure that the entire theme is GPL, or GPL-Compatible. This will include, fonts, scripts, images etc.

    Links:

    Once upon a time, we were slammed with spammers and even though that is no longer the case, we still need to verify the link accuracy and that would be Author URI and Theme URI.

    Trademarks:

    The trademark issues are usually found within the theme name. Most obvious names are easy to find: “WordPress”, “Google”, “Twitter” etc.

    Screenshot:

    Screenshot should be as accurate as possible.

    header.php:

    Usually I would look for any errors, such as hardcoded scripts or extra stylesheets (including Google Fonts).

    Also, plugin-territory codes and data validation issues.

    Now that we checked the header.php we would dive all the way down into the footer.php area.

    footer.php:

    Here we are going to repeat what we just did in header and for the logical reasons. In most cases if there are issues in header area, we will find them in footer too.

    The next stop is the brain of the theme and yes you guess it, functionality.

    functions.php:

    What to look for:

    If you are working with additional libraries and frameworks the process will be longer of course.

    • Proper theme setup function
    • Proper scripts and stylesheets inclusion
    • Proper content directory inclusion
    • Everything is prefixed and have unique text-domains
    • Everything is hooked properly
    • Potential security and privacy issues
    • Core features are not restricted or removed

    When the basics are covered, the next step is to follow everything that’s loaded in the functions.php i.e.

     


     

    And this was the very basic (not complete)  theme review process.

     

    If you have any questions please don’t hesitate to comment below.

    Hope that this helps :)

    Happy Holidays!

     
  • Tammie 3:49 pm on December 22, 2014 Permalink |  

    Introducing the new auto assigning button! 

    Christmas is really here for the theme review team as we’ve got our auto assigning button. Check it out in our sidebar!

    2014-12-22 at 15.42

    From today, we are retiring the comment queue and this will be the way new reviewers get themes. If you are an existing reviewer, nothing should change for you. This button is designed to work for those who haven’t got a theme yet. A big thanks to @nacin for making this much needed button happen.

    In the future we are going to also be assigning mentors, but that is in phase two for this button.

     
c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel