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.

Sign up for GitHub

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

Jump to bottom

chore: standardize npm script names per #14827 #16315

Merged
merged 11 commits into from Dec 14, 2022
Merged

chore: standardize npm script names per #14827 #16315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
88f5c45
chore: standardize npm script names per #14827
pmcelhaney Sep 15, 2022
fbdf6da
put lint:fix right after lint
pmcelhaney Sep 15, 2022
552072a
chore: fix ordering of :fix
pmcelhaney Nov 11, 2022
60bf6f8
chore: change release scripts to have "release:" prefix
pmcelhaney Nov 28, 2022
bb930b4
docs: update package.json naming to include 'release'
pmcelhaney Nov 28, 2022
c7007c3
docs: add css so abnf code formats okay
pmcelhaney Nov 28, 2022
ccd3370
chore: exclude linting from npm test
pmcelhaney Dec 8, 2022
a7efbeb
Merge branch 'main' into 14827-standardize-npm-scripts
pmcelhaney Dec 8, 2022
e445fcd
Revert "docs: add css so abnf code formats okay"
pmcelhaney Dec 13, 2022
c3e9628
Merge branch 'main' into 14827-standardize-npm-scripts
pmcelhaney Dec 13, 2022
9ee75d7
docs: remove counter-reset. I guess that was a merge error
pmcelhaney Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion Makefile.js
View file
Open in desktop
Expand Up @@ -632,7 +632,6 @@ target.karma = () => {
};

target.test = function() {
target.lint();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate linting from testing.

target.checkRuleFiles();
target.mocha();
target.karma();
Expand Down
30 changes: 14 additions & 16 deletions docs/src/developer-guide/package-json-conventions.md
View file
Open in desktop
Expand Up @@ -9,22 +9,16 @@ The following applies to the "scripts" section of `package.json` files.

npm script names MUST contain only lower case letters, `:` to separate parts, `-` to separate words, and `+` to separate file extensions. Each part name SHOULD be either a full English word (e.g. `coverage` not `cov`) or a well-known initialism in all lowercase (e.g. `wasm`).

Here is a summary of the proposal in EBNF.

```ebnf
name = life-cycle | main ":fix"? target? option* ":watch"?

life-cycle = prepare | preinstall | install | postinstall | prepublish | preprepare | prepare | postprepare | prepack | postpack | prepublishOnly;

main = "build" | "lint" | "start" | "test";

target = ":" word ("-" word)* | extension ("+" extension)*;

option = ":" word ("-" word)*;

word = [a-z]+;

extension = [a-z0-9]+;
Here is a summary of the proposal in ABNF.

```abnf
name = life-cycle / main target? option* ":watch"?
life-cycle = "prepare" / "preinstall" / "install" / "postinstall" / "prepublish" / "preprepare" / "prepare" / "postprepare" / "prepack" / "postpack" / "prepublishOnly"
main = "build" / "lint" ":fix"? / "release" / "start" / "test"
target = ":" word ("-" word)* / extension ("+" extension)*
option = ":" word ("-" word)*
word = ALPHA +
extension = ( ALPHA / DIGIT )+
```
Comment on lines +14 to 22
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a rough sketch working from memory. I looked up the grammar for ABNF and rewrote it correctly.


## Order
Expand All @@ -41,6 +35,10 @@ Scripts that generate a set of files from source code and / or data MUST have na

If a package contains any `build:*` scripts, there MAY be a script named `build`. If so, SHOULD produce the same output as running each of the `build` scripts individually. It MUST produce a subset of the output from running those scripts.

### Release

Scripts that have public side effects (publishing the web site, committing to Git, etc.) MUST begin with `release`.

### Lint

Scripts that statically analyze files (mostly, but not limited to running `eslint` itself) MUST have names that begin with `lint`.
Expand Down
28 changes: 14 additions & 14 deletions package.json
View file
Open in desktop
Expand Up @@ -13,22 +13,22 @@
"./use-at-your-own-risk": "./lib/unsupported-api.js"
},
"scripts": {
"build:docs:update-links": "node tools/fetch-docs-links.js",
"release:generate:latest": "node Makefile.js generateRelease",
"release:generate:alpha": "node Makefile.js generatePrerelease -- alpha",
"release:generate:beta": "node Makefile.js generatePrerelease -- beta",
"release:publish": "node Makefile.js publishRelease",
"release:generate:rc": "node Makefile.js generatePrerelease -- rc",
"build:site": "node Makefile.js gensite",
"build:webpack": "node Makefile.js webpack",
"lint": "node Makefile.js lint",
"lint:fix": "node Makefile.js lint -- fix",
"lint:docs:js": "node Makefile.js lintDocsJS",
"lint:fix:docs:js": "node Makefile.js lintDocsJS -- fix",
"test": "node Makefile.js test",
"test:cli": "mocha",
"lint": "node Makefile.js lint",
"lint:docsjs": "node Makefile.js lintDocsJS",
"fix": "node Makefile.js lint -- fix",
"fix:docsjs": "node Makefile.js lintDocsJS -- fix",
"fuzz": "node Makefile.js fuzz",
"generate-release": "node Makefile.js generateRelease",
"generate-alpharelease": "node Makefile.js generatePrerelease -- alpha",
"generate-betarelease": "node Makefile.js generatePrerelease -- beta",
"generate-rcrelease": "node Makefile.js generatePrerelease -- rc",
"publish-release": "node Makefile.js publishRelease",
Comment on lines -23 to -27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These scripts are used on Jenkins, so we need to decide if we'll make a change there or keep these names here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how complex / risky it is to update Jenkins. If you decide to use the new names, might want to do it like this.

  1. Add the new names (without removing the old names).
  2. Update Jenkins to use the new names.
  3. A second commit that removes the old names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s just leave these as-is right now. These aren’t just used within Jenkins, they are baked into eslint-release, which is used by all of our packages, making changing these script names more complex.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, was too tired when I wrote this. The only place using these command is in Jenkins, and so long as we update Jenkins before merging this, I don’t see a problem.

"gensite": "node Makefile.js gensite",
"webpack": "node Makefile.js webpack",
"perf": "node Makefile.js perf",
"docs:update-links": "node tools/fetch-docs-links.js"
"test:fuzz": "node Makefile.js fuzz",
"test:performance": "node Makefile.js perf"
},
"gitHooks": {
"pre-commit": "lint-staged"
Expand Down