New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite plugin #83

Closed
wants to merge 1 commit into
base: development
from

Conversation

Projects
None yet
1 participant
@dingo-d
Copy link
Member

dingo-d commented Feb 24, 2018

This PR is still a big WIP, and should not be merged.

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

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
  • Test the basic functionality
  • 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
  • Find out how to zip the plugin so that it works in the build script.
  • Change license to from MIT to GPL for compatibility with other WordPress projects
  • Once approved, rebase and merge to master branch

Review is welcomed as are suggestions.

@dingo-d

This comment has been minimized.

Copy link
Member Author

dingo-d commented Oct 27, 2018

Just a little update on what I did today:

I updated the npm scripts (babel and updated to latest versions) and the composer scripts to use WPThemeReview ruleset (still the dev version atm, until 0.1.0 is released).

I fixed the runner, and I'm getting an output (of sorts), which is a progress I guess.

When I run the sniffer against Twentyseventeen theme using the WPThemeReview sniff in the console I can see this as an output:

Registering sniffs in the WPThemeReview standard... DONE (132 sniffs registered)
{"success":true,"data":true}

I need to see if the config in the class-checks.phpis working - from the looks of it it's finally picking everything out as it should. What will be needed is a way to output the results like before. I just need to figure out how 😄

Oh and Travis is acting out, I need to see what's wrong with that.

@dingo-d dingo-d added this to the 0.2.0 milestone Nov 12, 2018

@dingo-d

This comment has been minimized.

Copy link
Member Author

dingo-d commented Nov 12, 2018

Quick update: I'm working on fixing the JS part of the sniffer. Note to self: Sort the phpcs JSON data in PHP so that the array returned (files) is an array that has file path and details as a part of an array because it's a bit hard to map through each one and create single item as it's currently implemented,

@dingo-d

This comment has been minimized.

Copy link
Member Author

dingo-d commented Jan 14, 2019

QA help from Danny Cooper:

  1. 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?
  2. 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.
  3. 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.
  4. Can't detect the licenses here for some reason - https://github.com/editorblocks/editor-blocks-theme/blob/bc5487ad0a4e6c08c85db86cf7b2732cf4f54e37/style.css#L1-L16

@dingo-d dingo-d changed the base branch from master to development Jan 15, 2019

@dingo-d

This comment has been minimized.

Copy link
Member Author

dingo-d commented Jan 15, 2019

Closing in favor of #97 (not working from fork)

@dingo-d dingo-d closed this Jan 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment