Theme Review Team

Recent Updates Page 2 Toggle Comment Threads | Keyboard Shortcuts

  • Ulrich 2:15 pm on October 1, 2016 Permalink |  

    Automation Meeting summary September 29, 2016 

    Items discussed

    You can view the full meeting notes in the Slack archive. A Slack account is required.

    @frankklein published a post on the background of the automation and why we have chosen ceratin tools. https://make.wordpress.org/themes/2016/09/29/why-are-we-working-to-improve-the-automation-of-the-theme-review/

    All of the active contributors gave a quick update what they have been working on.

    • @grapplerulrich is working on coordinating contributions and looking to modularize the restricted function sniffs upstream in WPCS.
    • @frankklein is working on underlying architecture/framework using phpDocumentor/Reflection library so that we can easily create sniffs to check the existence of files and functions as PHPCS is unable to do this. Codename `Theme_Analyser`
    • @kevinhaig has been working on automation every day since July. He has worked on `Abstract_ThemeSniff` and large number of other sniffs.
    • @jrf has been contributing upstream at WPCS and PHPCS to fix issues. She has also worked on utilities which should make the code for future sniffs easier.

    We have set a few goals for the next meeting

    • Get a MVP for the `Theme_Analyser` so that others can review the code and start extending it.
    • Get the easy pull requests ready so that they can be merged.
    • Merge as many PR as possible.

    The next meeting will be on the 13th October:

    Channel: #themereview | Time: Thursday October 13th at 14:00 UTC 14:00 UTC

  • Frank Klein 5:11 am on September 29, 2016 Permalink |
    Tags: review-automation   

    Why are we working to improve the automation of the theme review? 

    During the last few years, the WordPress Theme Review Team has had a hard time to keep up with the volume of submitted new themes.

    A high number of submissions is a good problem to have. But it means that developers have to wait a long time before their theme is published.

    A theme review includes a lot of routine tasks. Examples are checking for the use of deprecated functions, or lack of support for core WordPress features. A lot of these tasks do not need the intervention of a human reviewer, and could be made by software.

    The benefits of automation

    The Theme Check plugin was created in 2011, and is used to scan every theme uploaded to WordPress.org. It runs checks on the submitted theme, and blocks themes that fail checks for required items.

    The number of checks, and the issues detected by the checks have been improved over the years. But not all the requirements in the review guidelines are verified by the plugin. This leaves the task of verifying these requirements to the reviewers.

    In June 2016, the admins started a project to improve the automation of theme review tasks. The goal of this project is to automate as many routine tasks as possible.

    This automation has the following benefits:

    • By detecting more required items during the upload process, themes that fail elementary guidelines will not make it into the review queue. This will reduce the number of theme review tickets that get closed after a quick review due to fundamental flaws in the theme code.
    • By using better analysis tools, the number of issues missed by a reviewer will diminish. Some themes have large code bases, making it difficult to catch all issues. This is important in the context of security, as one flaw is often all that is needed to put an entire install at risk.
    • By using the new tool during the scans of automated theme updates, the quality of the themes will be kept at a high level.
    • By having software handle the menial work, reviewers can focus on areas where their expertise is needed, and provide a real benefit to theme authors.

    The flaws of the Theme Check plugin

    The Theme Check plugin has a fundamental flaw, and that is the reliance on text parsing for detecting errors.

    A text parser only understands text. It has no notion of what valid PHP code is, and what isn’t. This can mean that a check will pass, although the theme does not respect the guideline. A tool on which the reviewer cannot rely is a lot less useful, since double checking is needed.

    Additionally text parsing relies on regular expressions, which are difficult to write. This can lead to bugs, which in the worst case can block a valid theme from being uploaded. This leads to lack of trust in the tool by developers, who see it as a nuisance, rather than a useful tool.

    The unreliability, coupled with the absence of unit tests, make the Theme Check plugin difficult to maintain. The risk of unintentionally introducing regressions is too high.

    There have been attempts to use the PHP tokeniser API in checks to avoid text parsing. This API is a set of functions that provide an interface into the PHP tokeniser in the Zend Engine, which is the standard PHP interpreter.

    A tokeniser breaks text up into elements called tokens, which get passed to a lexer, that attaches meaning to the tokens. This means that after this operation, you can determine what a token represents, based on its internal type as used by the PHP interpreter.

    The problem with the current use of the PHP tokeniser in checks is that the API is too low level to be useful. Additionally transforming source code to tokens is an expensive, and therefore a slow operation.

    The current architecture of the Theme Check plugin does not offer a high level API to use the tokeniser in checks in a performant way. It needs to be rewritten from scratch, using better tools. After a discussion at WordCamp Europe between the theme review admins and other developers, using PHP_CodeSniffer seemed to be the best solution.

    A better approach, the PHP CodeSniffer

    PHP_CodeSniffer (PHPCS) is a static code analysis tool, meaning that it can analyse code without running it. PHPCS tokenises code, and runs sniffs on them. These sniffs serve to detect violations of a defined coding standard.

    PHPCS has coding standards for all major PHP projects, and WordPress is one of them, with a standard called WPCS.

    Using WPCS has four major advantages:

    1. The existing sniffs for the different WordPress coding standards give us a head start on detecting essential issues.
    2. PHPCS has offers a higher level API for interacting with the PHP tokeniser, making sniffs easier to write.
    3. With the WPTRT participating in the development of WPCS, there will be more contributors to the project. This tool is a crucial tool for the WordPress ecosystem. More developers means a bigger positive impact on WordPress as a whole.
    4. WPCS can be integrated with most editors, and integrated development environments (IDEs). PHPStorm is an example of an IDE with great support for PHPCS checks. This allows the tool to provide feedback while the developer writes code.

    The idea is to add a extra coding standard, WordPress-Theme, to the WPCS project. A list of sniffs that would need to be implemented as part of this project can be found on Github.

    As part of this project, @jrf has done a great job working on the base WPCS project. The long list of improvements in version 0.10.0 speaks for itself.

    Limits of PHP_Codesniffer

    The theme review guidelines can broadly be divided into two categories:

    1. Guidelines that cover technical aspects of theme development. An example would be lack of using the `eval()` function. PHPCS is great for detecting issues like this.
    2. Policy guidelines that are specific to a theme distributed on WordPress.org. An example would be a theme tagged with rtl that nonetheless lacks support for RTL languages. PHPCS is unfortunately not the right tool to detect these issues.

    This is due to the way that PHPCS works. The sniff process goes through the files one at a time, and runs all the sniffs on the current file. Once all files are processed, the sniff is considered complete. As such the tool has no knowledge of what the object of the sniff is. It just deals with files.

    Additionally, PHPCS sniffs detect errors by looking for certain combinations of tokens. So it’s up to the person writing the sniff to know which token pattern represents a function call for example.

    To effectively check the policy guidelines, we would need a tool specifically designed for the task. A theme is a collection of PHP, JavaScript, and CSS files, but we would need a tool that goes beyond this basic level.

    There is an existing PR on the WordPress-Theme standards repository, that extracts a set of data points from a theme. While the implementation itself is not a final solution, the approach has merit. Rather than dealing with individual files, the relevant information is extracted, and serves as an abstract representation of the theme.

    We are currently working on a test project that uses a PHP Parser to extract this information. A parser is one level above a lexer, as it turns the tokens into an abstract syntax tree. This is an advantage, because a parser knows how the tokens fit together.

    The library used is in this project is phpDocumentor/Reflection. This library was recommended by @rmccue, since the PHPDoc parser powering the WordPress Code Reference is based on it.

    The project is still in an early phase. It will be made available for contributions and testing as soon as a first stable version exists.

    How can you help?

    If you are a theme developer, start using the WordPress-Theme coding standard as part of your development process.

    The WPCS project in general, and the WordPress-Theme coding standard in particular, could benefit from the help of proficient developers.

    If you want to follow the advancement of the project, you can attend the automation meetings.

    As the guidelines are reviewed and adjusted regularly, make sure to attend the WPTRT meetings.

  • Carolina Nymark 3:34 am on September 29, 2016 Permalink |  

    Incomplete theme submissions 

    Lately we have received multiple requests on a daily basis to reopen themes that have been closed because there where more than five errors.

    It is your responsibility as theme authors to only submit themes that are complete, tested, and ready for review. We also expect you to stay up to date with the latest requirements and to keep your theme updated while in the queue.

    If your theme has not been updated for several months, we will not be able to reopen the ticket and grant you more time to fix the errors, -you already had several months to do this.  Even if your ticket has been closed, we are still available to answer your questions in ticket or in Slack.  We are hopeful that the waiting time will continue to decrease.

    Do not wait until a reviewer has been assigned: update your theme continuously. This is especially important when new versions of WordPress has been released after your first submission.   When a review starts, we review the latest version of the theme. If you submit a large update during the review, it means that we have to review your theme twice, and that will greatly increase the time it takes for your theme to go live.

    We are also seeing an increase of rushed and incomplete updates, where the reviewer has to repeatedly ask authors to fix different problems with the theme. You don’t need to rush your update: instead, take your time and make sure that the theme follows the requirements.  If you need more than seven days, let the reviewer know. If you are not sure if a request is recommended or required, ask the reviewer before you submit your update.

    The most common reasons to why we need to close themes include:

    The full list of requirements can be found here.

  • Jose Castaneda 7:59 pm on September 27, 2016 Permalink |  

    Review Meeting summary September 27, 2016 

    Items discussed

    Slack Link

    • Ammu themes 4:17 pm on September 28, 2016 Permalink | Log in to Reply

      What is Review Shindig? if you don’t mind 🙁

      • Jose Castaneda 9:45 pm on September 28, 2016 Permalink | Log in to Reply

        Our review shindig is a weekend of many of us theme reviewers conducting reviews on the same day. We’ll be on Slack for that weekend.

        The three organizers are going to be streaming live for some time: https://www.youtube.com/watch?v=0GSVmGi8wzM

        At the end of the week I will try and post stats about the event like how many themes were reviewed, how many people participated. Last event we managed to take the queue down by about 100 tickets I believe.

  • Ulrich 2:00 pm on September 27, 2016 Permalink |  

    Starting the automation meeting 

    After WordCamp Europe in June this year we started to work on the automation of the theme review requirements.

    So to be able to discuss the open issues and pull requests we will start with bi-weekly meeting on Thursday.

    Channel: #themereview | Time: Thursday at 14:00 UTC 14:00 UTC

    The agenda for the 29th September will be

    • Short update from the contributors what they are working on.
    • Define goals for the next meeting.

    Pinging @kevinhaig, @poena, @frankklein, @pross, @jrf

    If I have missed any usernames, it’s not on purpose and do consider yourself invited to the meeting

compose new post
next post/next comment
previous post/previous comment
show/hide comments
go to top
go to login
show/hide help
shift + esc
Skip to toolbar