use Grunt

[michaeltorbert]

from private PM

"for phplinting and php coding standards...
it uses NodeJS and runs locally. It does quite a few things - running PHPCS, minimizing CSS/JS, optimizing images, running text-domain validations, preparing the POT file and even ensures phpunit test cases are run in case they are configured. PHPStorm makes the integrator responsible for a lot of these things whereas by using grunt, this responsibility can be distributed to the contributors 😄 I find it useful because I don't have to worry about checking in improper code. I mentioned it because I see lots of code that does not conform to coding standards. I can set it up if you want."

[amostajo]

My little grain of salt... just so we can choice the right JS tasker, between grunt and gulp.
https://medium.com/@preslavrachev/gulp-vs-grunt-why-one-why-the-other-f5d3b398edc4
Let's discuss further on slack.

[contactashish13]

@michaeltorbert here are all the tasks that need to be performed:

1. we will need to create a new repo which will contain all the grunt plugins that we need to use. This will ensure that both the free and pro plugins can share the same set of tasks. I will create this repo on my account and you can clone it.
2. locally install node, npm and grunt
3. when I run grunt locally, it will change all the files according to the grunt tasks.

Do you want me to push changes only for task 1 above or also for 3? If for both, do you want me to create one single PR for both or would you prefer 2 different PRs?

[michaeltorbert]

@contactashish13 I think it's better if the grunt plugins are in the WP plugin repo.

[contactashish13]

@michaeltorbert i would suggest they be in a separate repository because they are going to be shared by the free and pro plugins. Also grunt tasks are not part of the plugin's functionality and this will unnecessarily complicate the grunt scripts.

Could you also respond to my question about the PRs?

[michaeltorbert]

@contactashish13 How would it complicate the grunt scripts? That should change them any.

-Let's hold off on any PRs until we've finished deciding exactly what to do.

[contactashish13]

@michaeltorbert is there a particular reason you want to keep grunt as part of the source code of the plugins despite it not adding any functionality to the plugin per se?

[michaeltorbert]

@contactashish13 What happens when something only applies to one? Or when we stop having Pro be effectively a fork of free?
It would only be in the development repository. Certainly we'd add it to .gitattributes to keep it out of releases.

[contactashish13]

@michaeltorbert Even if it applies only to one, it just seems more organized (following the separation of concerns paradigm) if these plugins are kept outside. What if tomorrow you want to give a grunt specialist access? You will have to give access to the entire plugin repo and not just grunt plugins.

[michaeltorbert]

@contactashish13 If we need a grunt specialist, I'm happy to give him access to what he needs. :)
Let's do it in the plugin repo for now, we can always make a separate one later.

[amostajo]

@contactashish13 Why do we need a new repo for this?
What do we want to accomplish with grunt?

I have used grunt and gulp many times in the past and didn't had the need for an extra repo. I know this repo might need some restructuring, but aside from that a new repo just for grunt makes no sense to me.

[contactashish13]

@amostajo I can't comment on those projects but in the projects that I have used grunt on, this is how the grunt specific part of package.json looks for every project

  "devDependencies": {
    "grunt-tasks": "<repo>",
    "load-project-config": "~0.2.0"
  },

this loads all the grunt plugins from the mentioned repo and uses load-project-config (https://github.com/cedaro/load-project-config) to load each grunt plugin into the task list. I don't find mixing grunt plugins with WP plugin a very organized method but if that's the way we want to go, I can work with that.

[contactashish13]

@michaeltorbert could you please let me know if there are still doubts about the implementation? If not, I can start working on this.

[michaeltorbert]

@contactashish13 You can go ahead and implement it in your fork of the repository and I'll take a look.

[amostajo]

@contactashish13 What tasks are stored in the separeated repo?

[amostajo]

Do you have an example?

[amostajo]

@michaeltorbert I have been looking at load-project-config and seems to be a NodeJS that has no support nor documentation what so ever. Last update done was on June 2016 and in order to understand what the package does and how to use it is, you have to do some reverse engineering to the main file.

This is clearly not a normal setup on any project, seems to be something specific done by a couple of people to setup their projects, we should use 3rd party packages that has at least documentation so any developer on the team can jump in and understand how to use them.

@contactashish13 This is an example of a JS package I, Social Shares, made 2 years ago, which uses grunt. You can see on the package.json that I make direct reference to the dependencies without having to create a middle tier repository as you are suggesting.

[michaeltorbert]

@amostajo Looks good.

[amostajo]

@contactashish13 similar thing on a greater scale project such as jQuery-UI, look at their package.json file, they don't use a middle tier repository for grunt tasks either.

Please provide an example to see what are you trying to accomplish, thanks.

[contactashish13]

@amostajo the approach used by jquery-ui and your previous project assume that the grunt tasks were to be used as-is. If any modification needs to be made to the task, the JS file that defines the code of the task needs to be stored somewhere. That is when https://github.com/cedaro/load-project-config is useful and, in the projects I have used, some tasks have been customized.

Maybe it would be better for us to decide on what grunt tasks are required for this plugin and whether their in-the-wild code is good enough and does not need any customization. That will help decide the course of action.

[amostajo]

Maybe it would be better for us to decide on what grunt tasks are required for this plugin and whether their in-the-wild code is good enough and does not need any customization. That will help decide the course of action.

@contactashish13 sounds very reasonable. What tasks are required @michaeltorbert ?

[michaeltorbert]

@amostajo I don't know, what did you and @contactashish13 have in mind?

[amostajo]

@michaeltorbert I though you and @contactashish13 had something in mind already...

The tasks that come to my mind are:

• SASS/LESS or similar (if you decide to use in the future)
• CSS minify
• JS minify
• Wordpress.org deployments via SVN push-commit
• Manage environments

Those tasks come to my mind atm.

[michaeltorbert]

@amostajo I didn't have anything specific in mind. We can make a list of everything, and maybe start off slow. :)
While I've wanted to look into using grunt for a while and hadn't gotten around to it, this all started when @contactashish13 mentioned it last month in a private chat.

[contactashish13]

@amostajo the chat came about because I saw that the code for pro was hard to read because the indentation was a bit messy and in some places even the curly braces were out of whack and I wondered if it would not be better to use grunt to prettify the code and also enforce PHPCS/WPCS. So the tasks that come to my mind are

phpcs/wpcs/phplint
jshint
minify css
minify js
automatic generation of POT files

[contactashish13]

@michaeltorbert I've checked in a few things relevant to this. For the time being, I'm using only 5 plugins - PHP coding standards, beautifier, linter and JS linter and uglifier. This takes care of 90% of what we might require.

Have checked in:

1. the package.json and phpcs.xml (as below)
2. the log file that phpcs generates when it runs (logs/phpcs.log): this will show you what phpcs tries to do and what it mandates needs to be done manually
3. the files that grunt has changed i.e. the php files it has beautified and linted and the js files it has minimized

Here is how to configure grunt and its dependencies:

0. install nodejs (https://nodejs.org/en/)
1. npm init (this will create package.json)
2. npm install grunt --save-dev (to install grunt)
3. npm install grunt-mkdir grunt-phpcs grunt-phpcbf grunt-phplint grunt-contrib-jshint grunt-contrib-uglify (to install the plugins)
4. (manually) create Gruntfile.js and phpcs.xml (already done)
5. run grunt

We can discuss this further once you review the PR: #1325

[contactashish13]

PR: #1591

This PR ignores whether phpdoc comments have been provided or not and only addressed code issues.

@michaeltorbert Can you please check if this fall through is deliberate as there was no break and grunt reported this? https://github.com/semperfiwebdesign/all-in-one-seo-pack/pull/1591/files#diff-a68f97d1d907ce30d561715c20d3e749R53

[contactashish13]

@michaeltorbert can we try to conclude on this so that going forward all PRs should have been run through grunt?

[michaeltorbert]

@contactashish13 What happens for PRs that haven't been run through grunt, ie when a guest contributor creates a PR?

[contactashish13]

@michaeltorbert they will have to be run through grunt before merging.

[contactashish13]

@michaeltorbert wrt our conversation on Slack, I had already mentioned #1003 (comment) :)

Another alternate way to go about handling guest PRs is by running grunt through travis only on the master branch. I'm not a big fan of this as, I believe, the master build publishes the wp.org and should be a fire-and-forget deployment. If we put grunt only there, someone will have to monitor that build and, if it fails (which it most likely will if grunt has not run on the PR before), pull the branch on a local dev, run grunt, fix issue, then merge again. Now this merge should never be on the master branch and should be on a dev/release branch. So this just increases the turn around time.

I would, instead, prefer that guest PRs (not that they are so numerous any ways) be merged locally which they already are for testing and code review, run grunt, fix issues and then merge to dev. This is a much safer process.

Again, using grunt is painful only the first time around, like what @EkoJR has been doing for #1607 (wpcs is just one part of the fix). Once the code base is fixed (which it hasn't since it was born), incremental changes will be seamless.

@EkoJR can you please confirm you have tested the droplet and things run for you smoothly there?

[arnaudbroes]

... so the tasks that come to my mind are:

• phpcs/wpcs/phplint
• jshint
• minify css
• minify js
• automatic generation of POT files

I don't want to crash the party, but can anyone explain to me why these things can't be done with a few plugins in PHPStorm?

[amostajo]

Got notified about this, just dropping a little grain of salt here... @arnaudbroes forcing the entire development and team members to only work with one IDE is not a good idea, more knowing that IDEs such as PHPStorm are not very efficient with the computer's resources (RAM and CPU consumption).

This is why it is better to have a solution, like grunt or gulp, which is cross-platform and can be used with any IDE. It has become pretty much a standard now days to use dependency injection and java taskers to maintain and develop repositories.

Regards

[michaeltorbert]

@contactashish13
Master doesn't automatically push to .org. Commits to the .org repo are a manual process.

[contactashish13]

@michaeltorbert that can be automated through travis and a droplet as well.

@EkoJR can you please confirm #1003 (comment)?

[EkoJR]

@contactashish13 As far as I know, I already confirmed it back on the 1st of October (2018) via Slack

[contactashish13]

@EkoJR cool, I couldn't find it anywhere. @michaeltorbert everything has been set up and tested by me and Ben. I think you need to test as well if you haven't already (the instructions are on slack).

After that we need to actually implement it as part of our regular dev cycle.

[contactashish13]

@michaeltorbert as discussed on the call, can you please test? The details were mentioned on slack some time ago. Can you also create a new branch called grunt so that it can be used for the grunted changes?

@EkoJR can you run grunt and then create a PR for @michaeltorbert to merge (to that grunt branch) so that this part of the process is clear?

[michaeltorbert]

@contactashish13 I don't need to test it if you're confident, and Ben has also tested. We can go ahead and move forward with this. Make sure there are clear guidelines/instructions on the dev process moving forward. ie you are Ben or whomever needs to run grunt at some point like we talked about on new PRs.

[contactashish13]

PR #2171 contains the files that were changed by grunt.

The changes might seem all over the place but if you look at the files and the changes, you will see that there are no breaking changes. Unit tests also confirm that independently but human eyes would be better to satisfy oneselves 😄

I have created a new branch on the main repo called grunt. The code can be merged into that branch and then tested to ascertain nothing breaks.

The current changes are mostly cosmetic changes to improve the overall readability of the code. Many rules dealing with proper code commenting have been excluded for the time being and can be (/will be) activated later to ensure code comments are in place as well.

@michaeltorbert @EkoJR @wpsmort please check and let me know if you find anything missing.

The next steps, after validation of this. could be:

• Create a workflow for guest PRs on the instance
• integrate grunt in travis
• start adding code comments per file so that WPCS does not fail when those rules are removed from phpcs.xml