WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
270 stars 3 forks source link

Feature/use namespace #97

Closed dingo-d closed 5 years ago

dingo-d commented 5 years ago

The aim is to have a modern, clear-cut development process that will help with future development of the plugin.

Note

I have closed the old PR because I want everything to be handled from this repo, and not my fork (easier to maintain).

Added

To do

Review is welcomed as are suggestions.

Help from testers

From Danny Cooper:
From Liton Arefin:

Getting error while activation.

Parse error: syntax error, unexpected ‘:’, expecting ‘;’ or ‘{‘ in “/theme-sniffer/src/class-plugin.php” on line 153

This is fixed.

From metallicarosetail (via slack)

Error while activation on PHP 7.0.12

Fatal error: Uncaught TypeError: Return value of Theme_Sniffer\Assets\Assets_Handler::add() must be an instance of Theme_Sniffer\Assets\void, none returned in /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/assets/class-assets-handler.php:35 Stack trace: #0 /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/assets/assets-awareness-trait.php(41): Theme_Sniffer\Assets\Assets_Handler->add(Object(Theme_Sniffer\Assets\Script_Asset)) #1 /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/admin-menus/class-base-admin-menu.php(33): Theme_Sniffer\Admin_Menus\Base_Admin_Menu->register_assets() #2 /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/class-plugin.php(137): Theme_Sniffer\Admin_Menus\Base_Admin_Menu->register() #3 [internal function]: Theme_Sniffer\Core\Plugin->Theme_Sniffer\Core{closure}(Object(Theme_Sniffer\Admin_Menus\Sniff_Page), 2) #4 /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/class-plugin.php(138): array_walk(A in /Applications/MAMP/htdocs/review/wp-content/plugins/theme-sniffer/src/assets/class-assets-handler.php on line 35

This is fixed. Removed void return types

dingo-d commented 5 years ago

@Danny-Cooper regarding the two issues you mentioned, the issue with the prefix, shouldn't they be named with underscore? editor_blocks?

The second issue with the licenses I don't think the reported is an error, since it detected that the license is present and that's ok. I could add a check for a type of license, but I can leave that for later.

What did work was the issue that you have theme in your theme name, and that's not allowed 🙂

Thanks for your contributions btw 🙂

DannyCooper commented 5 years ago

@dingo-d I was using the underscore, but the sniffer seemed to want a hyphen?

If a theme name has dashes e.g 'editor-blocks' it expects variables to be $editor-blocks_variable and won't accept $editor_blocks_variable.

dingo-d commented 5 years ago

@DannyCooper can you check the thing with the theme prefix? When I tried with editor_blocks I got some errors like

Variables defined by a theme/plugin should start with the theme/plugin prefix. Found: "$comments_number".

If I add editor-blocks as a prefix I get

WARNING  The "editor-blocks" prefix is not a valid namespace/function/class/variable/constant prefix in PHP.
joyously commented 5 years ago

You can't have a prefix with a dash. PHP will parse it as a subtraction.