Skip to content
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

Standardize script naming in main package.json #19949

Closed
mkaz opened this issue Jan 29, 2020 · 8 comments · Fixed by #42368
Closed

Standardize script naming in main package.json #19949

mkaz opened this issue Jan 29, 2020 · 8 comments · Fixed by #42368
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling

Comments

@mkaz
Copy link
Member

mkaz commented Jan 29, 2020

During development of #19855, we struggled with the naming of new scripts, and realized there wasn't quite a standard for how scripts should be named within package.json. For example, when and why to use dash to separate parts, or when to use :

@gziolo suggested:

We could bring some structure to the names of all the existing scripts in the main package.json file as an independent patch smile

This ticket opens the discussion for it so we don't forget to circle back on it.

@gziolo gziolo added [Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling labels Jan 29, 2020
@gziolo
Copy link
Member

gziolo commented Mar 10, 2020

Sharing a related tool that could be used as a baseline for the patterns: https://github.com/peerigon/scriptlint.

The rules they offer for the package.json's "scripts" section:

  • have a test script that is not the default script from npm init
  • have a dev script and a start script
  • abstract script names from their implementation (test, not jest)
  • use namespaces to categorize scripts ("test:unit": "jest")
  • use : as a namespace separator
  • have the scripts in alphabetic order
  • have a trigger script for all hooks (ex: if you have prefoobar, there must be a foobar script)
  • use camelCase for all script names
  • not alias devDependencies (no "jest": "jest")
  • not use && or & for sequential or parallel script execution

(italic = strict rule)

@gziolo gziolo added Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts labels Feb 1, 2021
@gziolo
Copy link
Member

gziolo commented Jul 11, 2022

@t-hamano, now that you wrapped up all tasks to make tooling work with Windows OS, do you think we would benefit from standardizing script naming in package.json? I'm on the fence now because those commands are documented in so many places and they have been around for so long.

@t-hamano
Copy link
Contributor

do you think we would benefit from standardizing script naming in package.json?

Yes, I agree with you.

However, since npm script is used in various places (including GitHub Actions), I wonder how to proceed.
Which is better to standardize scripts in one PR or in stages?

@gziolo
Copy link
Member

gziolo commented Jul 11, 2022

However, since npm script is used in various places (including GitHub Actions), I wonder how to proceed.
Which is better to standardize scripts in one PR or in stages?

I hope it's possible to do it one PR and keep old aliases for some transition period.

@t-hamano
Copy link
Contributor

I see, is this what you mean by keeping old aliases ? 🤔

{
	"name": "gutenberg",
	"version": "13.6.0",
	"scripts": {
		//new scripts
		"new-dev": "npm run build:packages && concurrently \"wp-scripts start\" \"npm run dev:packages\"",
		"new-build": "npm run build:packages && wp-scripts build",
		// old scripts
		"dev": "npm run new-dev",
		"build": "npm run new-build"
	}
}

@gziolo
Copy link
Member

gziolo commented Jul 11, 2022

I see, is this what you mean by keeping old aliases ? 🤔

Yes, that would work as a temporary solution for a few months 👍🏻

@t-hamano
Copy link
Contributor

t-hamano commented Jul 12, 2022

Ok, then I would like to work on this issue.
I would like to temporarily install scriptlint and start by scanning the current script.

@gziolo
Copy link
Member

gziolo commented Aug 8, 2022

@t-hamano, excellent work tackling this task ❤️

@t-hamano t-hamano removed [Status] In Progress Tracking issues with work in progress Needs Dev Ready for, and needs developer efforts labels Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants