• Resolved Sayontan Sinha

    (@sayontan)


    I just downloaded the latest version of the plugin and I see a few things that are potentially concerning:
    1. As per the the discussion on the Theme Reviewers’ site the use of add_theme_page is still a recommendation. But the plugin now prints out a “warning” if there are calls to add_submenu_page. Shouldn’t this be a “recommendation” instead?

    2. The check for exec seems to be broken. It is reporting curl_exec, executed, and calls to functions like shell_exec as warnings, even if some of these calls are commented out.

    http://wordpress.org/extend/plugins/theme-check/

Viewing 9 replies - 1 through 9 (of 9 total)
  • The discussion from the WPTRT make themes sites with the dev’s lead to the conclusion that add_theme_page is to be used. Not as a recommendation but a requirement. It was brought up as a thought to be a recommendation but it turned into a requirement at the end.

    The reason it’s in the theme_check plugin early is to prepare the developer that its a requirement.

    Thread Starter Sayontan Sinha

    (@sayontan)

    @frumph,
    I did reference that discussion in my comment above. My understanding is based on the last comment on that thread:

    Agreed. The gist of this discussion should be showing up as a recommendation to coincide with the 3.1 release. We can then work from that benchmark furthering the requirements with 3.2.

    So isn’t this still supposed to be a recommendation? And we are still in 3.0.4. The reason I ask is because my understanding is that if something shows up as a “warning” in the theme check plugin, it becomes grounds for rejection of the theme, is it not?

    Right, Cais mispoke the recommendation word with requirement. I made a reply under him to make it apparent of requirement.

    ^ As for the grounds for rejection, I know you, you’ll point it out to the reviewer that it’s not grounds for rejection until 3.1 which would be completely valid.

    However, since you know it’s going to be grounds for rejection eventually, what’s stopping you from changing? Is there an issue that we should discuss why it would still be appropriate to keep using add_submenu_page ?

    If so, you should comment on that post over at the http://make.wordpress.com/themes/ site to show your concerns, you can login via your normal username and password.

    Thread Starter Sayontan Sinha

    (@sayontan)

    Is there an issue that we should discuss why it would still be appropriate to keep using add_submenu_page ?

    The main issue here stems from the number of options on my theme. Suffusion happens to be one of those themes that has sub-menus, and then it has tabs under sub-menus.

    Though I have a personal dislike for the eventual output, I don’t mind going the route of the theme installation page if it needs to be done (tabs at the top instead of sub-menus). It is just that I am finishing up putting final touches on a massive redesign using the Settings API (a rather painstaking exercise – I don’t believe there is any theme with these many options that has been handled using the Settings API) and getting rid of TimThumb completely. Adding this to the mix would only increase the testing complexity of this release even further.

    We discussed that as well, and found that there’s no problems with adding multiple add_theme_pages for your theme for sep. areas of options without using tabs, the idea is to keep options in a specific location instead of using add_menu_page/add_submenu_page in other locations other then the appearance section.

    *if* however you needed to put something under settings -> tools for example – if any other sections then we really would like to know your opinion on the matter so speaking up on the make.wordpress.org/themes site on the post would be great for that.

    Most of the instances that have been reviewed for that sort of issue have been thought of as plugin territory but if you have other idea’s.

    ^ Awesome about the settings API, although I’m not looking forward to doing mine either – frankly I’m against it being a requirement in the future and will be verbal about it; unfortunately it’s not my decision to make =( just to give my opinion about only.

    mea culpa, mea culpa … Frumph is correct, I “mispoke” the word recommendation for requirement and have since edited and added an additional reply noting the difference.

    The themes in the repository are required to perform well in all the common situations and in most of the probable situations. Sometimes compromises must be made to meet this requirement.

    Think about this one the following way:

    • A separate location for the theme options group is confusing, especially if it’s alone at the end of the menu
    • If another child theme/plugin/component/extension claims the position of that top menu item, either yours or the other one is gone, how’s that for usability?
    Thread Starter Sayontan Sinha

    (@sayontan)

    Sometimes compromises must be made to meet this requirement.

    I am not against doing a compromise (though I don’t like the final look that you will have when you use add_theme_page for so many options). I was just questioning its inclusion as a requirement for version 3.0.4 as opposed to 3.1 as agreed in the reviewers’ discussion.

    A separate location for the theme options group is confusing, especially if it’s alone at the end of the menu

    That’s really a question of taste and organization. If you look at the commercial theme marketplace you will see several themes with a separate section, and not necessarily for branding – some themes do a lot more stuff than others. At the end of the day all talk of “confusion” is a one-time thing for most users. After all people do adapt to plug-ins that introduce new menu items (e.g. GD Star Ratings)

    If another child theme/plugin/component/extension claims the position of that top menu item, either yours or the other one is gone, how’s that for usability?

    If you add a menu item without a position argument it gets added to the bottom of the menu, and any number of themes or plugins could use a blank argument – they just would get stacked at the bottom of the menu one after the another. While I cannot control where someone else puts his/her menu, I surely can add mine using a blank position argument – that way I don’t take anybody else’s position and somebody else doesn’t take mine due to a conflicting position argument. This to me is a non-issue.

    Plugin Author Simon Prosser

    (@pross)

    In reply to the OP this commit fixes the exec() issue.

Viewing 9 replies - 1 through 9 (of 9 total)
  • The topic ‘[Plugin: Theme-Check] Questions about the new version’ is closed to new replies.