Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

Feature/use namespace #97

Merged
merged 59 commits into from
Mar 4, 2019
Merged

Feature/use namespace #97

merged 59 commits into from
Mar 4, 2019

Conversation

dingo-d
Copy link
Member

@dingo-d dingo-d commented Jan 15, 2019

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

  • Namespaces
  • Class autoloading
  • Build scripts
  • Updated dependencies in the packages
  • Rewrote the dev plugin structure

To do

  • Link to theme sniffer page in the plugins page is not showing
  • Increase the sniff speed - limit sniffing to php files and use eslint and stylelint for .js and .css files respectively
  • Rewrite the HTML and CSS part so that BEM methodology is used
  • Test the basic functionality
  • Find out how to zip the plugin so that it works in the build script
  • Once approved, rebase and merge to master branch
  • Add a check for PHP files only
  • Pull tags from https://api.wordpress.org/themes/info/1.1/?action=feature_list and put them in a transient (pull every X days)

Review is welcomed as are suggestions.

Help from testers

From Danny Cooper:
  • If no 'Standard' is selected, it still looks like it's working and you have to press stop. Might be better to not start at all if no boxes are checked?
  • If you type into 'Theme prefixes' and press return, it refreshes the page. It would be more intuitive if it did nothing or started the sniffs.
  • If a theme prefix has dashes e.g 'editor-blocks' it expects variables to be $editor-blocks_variable and won't accept $editor_blocks_variable.
  • Can't detect the licenses here for some reason - https://github.com/editorblocks/editor-blocks-theme/blob/bc5487ad0a4e6c08c85db86cf7b2732cf4f54e37/style.css#L1-L16
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 dingo-d added Type: Enhancement This is an enhancement of the existing features or a new feature to the plugin. Status: Help Wanted A help is needed regarding an issue or an opened PR. pr: needs update Status: Future Release This issue will be fixed in the future release of the plugin, or an enhancement will be added in the labels Jan 15, 2019
@dingo-d dingo-d added this to the 0.2.0 milestone Jan 15, 2019
@dingo-d dingo-d self-assigned this Jan 15, 2019
@dingo-d dingo-d mentioned this pull request Jan 15, 2019
12 tasks
Props to Danny Cooper for testing this out.
Props to Danny Cooper for testing this out and catching a bug.
@dingo-d
Copy link
Member Author

dingo-d commented Jan 19, 2019

@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
Copy link

@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.

timelsass and others added 28 commits February 28, 2019 23:46
Update the sniff page to catch the exception and handle it gracefully on the admin page.
@dingo-d dingo-d merged commit d70c0b5 into development Mar 4, 2019
@dingo-d dingo-d deleted the feature/use-namespace branch March 5, 2019 07:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Future Release This issue will be fixed in the future release of the plugin, or an enhancement will be added in the Status: Help Wanted A help is needed regarding an issue or an opened PR. Type: Enhancement This is an enhancement of the existing features or a new feature to the plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants