Scripts: New package with scripts for plugins by gziolo · Pull Request #55 · WordPress/packages

gziolo · 2017-12-19T16:41:30Z

Still work in progress ...

This implementation is based on the react-scripts. Grunt tasks are copied directly from wp scaffold plugin templates. See gruntfile for reference.

The end goal would be to run it as:

wp-scripts i18n --pluginSlug=abc --textDomain=xyz
wp-scritps readme

To include you would have to include in devDependencies only this:

"@wordpress/scripts": "1.0.0"

Disclaimer

I'm not promoting Grunt by any means. It was easy to copy what we already have in wp-cli to show how it could benefit plugin developers. Script names could be prefixed with plugin- in this case to make it more clear where it is applicable. This could also live in plugin-scripts package to avoid confusion.

codecov · 2017-12-19T16:44:11Z

Codecov Report

Merging #55 into master will decrease coverage by 15.47%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master      #55       +/-   ##
===========================================
- Coverage   97.29%   81.81%   -15.48%     
===========================================
  Files          17       19        +2     
  Lines         148      176       +28     
  Branches       41       48        +7     
===========================================
  Hits          144      144               
- Misses          4       25       +21     
- Partials        0        7        +7

Impacted Files	Coverage Δ
packages/scripts/bin/wp-scripts.js	`0% <0%> (ø)`
packages/scripts/Gruntfile.js	`0% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c58b48a...c1ad274. Read the comment docs.

gziolo · 2017-12-19T16:44:17Z

@danielbachhuber, this is the proof of concept I wanted to share with you :)

ntwb · 2017-12-20T00:02:56Z

FYI: WordPress' official build tool is Grunt, and we've recently added Webpack. Official WordPress repos should use Grunt over Gulp, the reason for this is primarily to reduce the required volume of learning for contributors to WordPress projects.

ntwb · 2017-12-20T00:05:32Z

packages/a11y/package.json

  },
  "dependencies": {
-    "@wordpress/dom-ready": "^0.1.0-beta.6"
+    "@wordpress/dom-ready": "0.1.0-beta.6"


This change shouldn't be in this PR should it?

Likewise the dom-ready/package-lock.json and a11y/package-lock.json changes shouldn't be in this PR either?

Yes, I will remove those changes.

gziolo · 2017-12-20T07:37:59Z

FYI: WordPress' official build tool is Grunt, and we've recently added Webpack. Official WordPress repos should use Grunt over Gulp, the reason for this is primarily to reduce the required volume of learning for contributors to WordPress projects.

The main goal of this package is to make it an implementation detail if WordPress is using Grunt, Gulp, Webpack or whatever is necessary to make things done. I can envision using all of those tools in this package, as well as having them replaced in the far future with something that is yet to be created. This approach promotes usage of npm run-script which is a well-established pattern in the JS community.

danielbachhuber · 2017-12-20T20:43:54Z

Can I use this and a Gruntfile in my own project?

e.g. this project would expose some wp-scripts readme I can run, but then I can continue to use my grunt styles for compiling SASS.

aaronjorbin

Loving this! Thanks for kicking it off.

aaronjorbin · 2017-12-20T21:09:20Z

package.json

  "scripts": {
    "build-clean": "rimraf ./packages/*/build ./packages/*/build-module",
    "build": "node ./scripts/build.js",
-    "postinstall": "lerna bootstrap --hoist && npm run build",


Is removing --hoist intentional?

Yes, I opened an issue to address it separately #56. I will rebase once it gets merged.

aaronjorbin · 2017-12-20T21:12:01Z

packages/scripts/Gruntfile.js

+
+	const pkg = grunt.file.readJSON( 'package.json' );
+	const pluginSlug = grunt.option( 'pluginSlug' ) || pkg.name;
+	const textDomain = grunt.option( 'textDomain' ) || '';


Should we start a standard of allowing package.json to contain a WordPress object that initially has textDomain and pluginSlug properties in it? pluginSlug could fallback to pkg.name if pkg.wordpress.pluginslug isn't defined.

I'm more than happy to do it. I thought it might be too early, but if you raised that now let's go for it 💯

aaronjorbin · 2017-12-20T21:12:45Z

packages/scripts/Gruntfile.js

+	const pluginSlug = grunt.option( 'pluginSlug' ) || pkg.name;
+	const textDomain = grunt.option( 'textDomain' ) || '';
+
+	console.log( pluginSlug, textDomain );


Leftover Dubugging code?

Yes, sir! Removing it :)

aaronjorbin · 2017-12-20T21:14:32Z

packages/scripts/Gruntfile.js

+		},
+
+		wp_readme_to_markdown: {
+			your_target: {


This is another area where pkg.wordpress could come in handy, since it would allow screenshots to be passed in.

aaronjorbin · 2017-12-20T21:16:37Z

packages/scripts/README.md

+Install the module
+
+```bash
+npm install @wordpress/scripts --save


The PR summary contains the line: "To include you would have to include in devDependencies only this:"

Would --saveDev make more sense?

aaronjorbin · 2017-12-20T21:28:12Z

When this is available, let's make sure to announce it on https://make.wordpress.org/core and cross-post it on https://make.wordpress.org/plugins/

gziolo · 2017-12-21T13:15:44Z

@adamsilverstein can we cover it with tests in some simple way?

danielbachhuber · 2017-12-21T13:17:10Z

Probably worth mentioning wp-cli/ideas#68

Sorry I didn't connect the dots before. Given wp makepot will soon exist, would it make sense to create a README command too and keep this entirely to PHP?

youknowriad · 2017-12-21T13:21:49Z

packages/scripts/package.json

+    "cross-spawn": "5.1.0",
+    "grunt": "1.0.1",
+    "grunt-wp-i18n": "1.0.1",
+    "grunt-wp-readme-to-markdown": "2.0.1"


Who maintains these dependencies, since now we're trying to build a utility hiding implementation details, we can try to move away from React, I think these would benefit from being simple Javascript Scripts instead of Grunt tasks.

Not saying we should do it now, the scripts module also allows us to keep using Grunt and several other tools and hide the implementation, but avoiding useless dependencies like "grunt" would be good (Same way there's babel, and grunt-babel etc...)

@stephenharris is the maintainer.

In general, I agree that lessening the dependency on Grunt makes sense, but at the same time it's not exposed to the plugin developer, so it is just an implementation detail rather than an api detail.

gziolo · 2017-12-21T13:23:26Z

Sorry I didn't connect the dots before. Given wp makepot will soon exist, would it make sense to create a README command too and keep this entirely to PHP?

So this would make this PR obsolete. I'd be more than happy to have that handled by wp-cli exclusively. Let me know if I should close this PR and open another one focused on exposing JS and CSS linting for plugins and themes.

gziolo · 2017-12-21T13:26:34Z

Can I use this and a Gruntfile in my own project?

e.g. this project would expose some wp-scripts readme I can run, but then I can continue to use my grunt styles for compiling SASS.

Yes, it doesn't stop you from having your own Grunt setup in your project. The nice thing about npm is that you can use several versions of the same dependency. I guess you would have to set grunt as a local dependency and all Grunt plugins to make it work.

aaronjorbin · 2017-12-21T16:16:40Z

Sorry I didn't connect the dots before. Given wp makepot will soon exist, would it make sense to create a README command too and keep this entirely to PHP?

I would support this. If wp-cli can be the tool of truth, I think it makes sense.

gziolo · 2018-01-09T12:29:50Z

Closing in favor of #62.

gziolo added Technical Prototype [Status] In Progress labels Dec 19, 2017

gziolo self-assigned this Dec 19, 2017

gziolo requested a review from adamsilverstein December 19, 2017 16:41

gziolo requested review from aduth and youknowriad December 19, 2017 16:51

ntwb reviewed Dec 20, 2017

View reviewed changes

Scripts: initial commit with scripts for plugins

c1ad274

gziolo force-pushed the add/scripts-for-plugins branch from f9f803d to c1ad274 Compare December 20, 2017 07:54

danielbachhuber mentioned this pull request Dec 20, 2017

Support scaffolding with Gulp wp-cli/ideas#60

Closed

aaronjorbin suggested changes Dec 20, 2017

View reviewed changes

gziolo mentioned this pull request Dec 21, 2017

Update Lerna and Jest to the latest version #56

Merged

youknowriad reviewed Dec 21, 2017

View reviewed changes

danielbachhuber mentioned this pull request Dec 22, 2017

Generate README.md from readme.txt for plugins wp-cli/ideas#74

Closed

gziolo closed this Jan 9, 2018

gziolo deleted the add/scripts-for-plugins branch January 9, 2018 12:29

gziolo mentioned this pull request Jan 9, 2018

Scripts: New scripts package containing test command #62

Merged

Conversation

gziolo commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Disclaimer

Uh oh!

codecov bot commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gziolo commented Dec 19, 2017

Uh oh!

ntwb commented Dec 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gziolo commented Dec 20, 2017

Uh oh!

danielbachhuber commented Dec 20, 2017

Uh oh!

aaronjorbin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronjorbin commented Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gziolo commented Dec 21, 2017

Uh oh!

danielbachhuber commented Dec 21, 2017

Uh oh!

youknowriad Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gziolo commented Dec 21, 2017

Uh oh!

gziolo commented Dec 21, 2017

Uh oh!

aaronjorbin commented Dec 21, 2017

Uh oh!

gziolo commented Jan 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gziolo commented Dec 19, 2017 •

edited

Loading

codecov bot commented Dec 19, 2017 •

edited

Loading

aaronjorbin commented Dec 20, 2017 •

edited

Loading

youknowriad Dec 21, 2017 •

edited

Loading