WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
208 stars 38 forks source link

[New sniff] WC template versions #203

Closed timelsass closed 5 years ago

timelsass commented 5 years ago

[New sniff] woocommerce template versions

Discussed originally on https://github.com/WPTRT/theme-sniffer/issues/116 for reference.

It would be nice have a sniff to detect outdated templates in woocomerce template overrides.

Basic Info -
Rule type: Error / Warning
Sniff Category: Plugins

Rule:

I don't think the handbook has any rules for wooCommerce and theme submissions that I could find - feel free to add on to this is anyone knows of some. I do believe if a theme declares support for something like that, that it should be at least an up to date implementation - similar to what the TGMPA sniff is doing.

Notes for implementation:

joyously commented 5 years ago

This has nothing to do with theme review. I don't think it belongs here.

dingo-d commented 5 years ago

How come? WooCommerce templates are directly related to themes...

joyously commented 5 years ago

The versioning of anything in the theme has nothing to do with theme review, except perhaps TGMPA itself. It's not a part of review to determine the if the version of Bootstrap or Font Awesome or Woocommerce templates are the latest. Perhaps the intent is to support legacy installations? This is out of scope.

justintadlock commented 5 years ago

I have to say I agree with @joyously on this one.

If this is something that folks want added, it should probably be under the theme review guidelines first.

timelsass commented 5 years ago

I would agree that it should be under the theme review guidelines. If a theme is including code in relation to other plugins, it probably should be part of the review process to ensure with those plugins active there aren't fatals or notices. I've encountered several times an outdated template in woocommerce results in php fatals/notices on frontend.

I don't agree that it's out of scope though, something like bootstrap and fontawesome, yeah those would be, but woocommerce directly deals with the frontend presentation, requires php code to be added in WordPress themes, and goes untested during reviews. Keeping those template files up to date for overrides is just one of the ways that it could at help theme authors are submitting quality themes from the beginning.

At some point the buddypress tag was added for themes, so someone else felt the same way about something that is so intrusive to the frontend of a site/theme. Given that buddypress has 200k+ installs compared to woocommerce at 4m+ installs, I think it deserves to have something in place to reduce chances of errors.

Even though these standards are used for the WordPress review process, many people will use these standards outside of .org for places like envato, or even just client sites. It's just one additional way to increase the quality of themes being delivered to end users who often times don't know what to do when they get fatals or notices.

Plus - the theme checks are already scanning woocommerce templates folder - so if the decision is to not have anything to do with woocommerce and not adding these checks, the review process and theme -sniffer should then skip this folder for things like globals checks etc that they common have in their templates. The process is saying you have to conform to the theme review standards, but we don't care if the end result is that user's sites don't work because there's a template that causes fatals now.

joyously commented 5 years ago

I would rather see a warning for non-core post types or something. It seems to me that if a plugin creates a post type, it should provide the output of it (template). We should be encouraging theme authors to provide easy ways to handle any post type instead of providing templates for a specific post type from a popular plugin. I can see a theme overriding CSS classes for woocommerce, but I think template files belong in the plugin.

dingo-d commented 5 years ago

I'll mark this as wontfix as it is kinda out of the scope for the TRT (at the moment). If the need arises, we can reopen this one.