search

WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 487 forks source link

Unify the composer testing scripts under one 'namespace'? #2115

Closed dingo-d closed 1 year ago

dingo-d commented 1 year ago

Is your feature request related to a problem?

Should we maybe put all the scripts that are used for testing purposes (lint checks, cs checks, tests, check-complete, etc.) under one namespace (say test)?

That way all the commands would be nicely grouped together when typing the composer command like this:

image

In addition to the above, we can also group them in one test command so that we can run just the composer test to run all of them which can be useful when working on sniffs.

Describe the solution you'd like

The following scripts: lint, lint-ci, check-cs, run-tests, check-complete or check-complete-strict would be prefixed with test: prefix

And we'd add the following script to the scripts list as well:

"test": [
  "@test:lint",
  "@test:lint-ci",
  "@test:check-cs",
  "@test:run-tests",
  "@test:check-complete-strict"
]

That would run all the 'tests' at once when the composer test is executed.

The impact is minimal, as only the workflows and Changelog would need to be updated (not sure if we have anything in the wiki, but that is also easy to update).

We could also add script descriptions to explain what each command is doing.

Any thoughts?

jrfnl commented 1 year ago

I couldn't care either way as I don't use the scripts.

GaryJones commented 1 year ago

I'm not too bothered, but composer test:check-cs is a lot longer than just composer cs. Any CLI auto-complete then needs extra work; will pause at test: rather than actually autocompleting where as composer l followed by tab would likely have the lint-ci option as one of the only commands.

Namespaces don't really matter too much, because we;re not going to be using the same name as an existing Composer command or Composer extension/plugin. If namespacing was important, then it we'd want a wpcs prefix, and then it starts getting silly.

Unless you feel particularly strongly, I'd leave the prefixes out.

You can still have composer test as a catch-all command without adding prefixes.

dingo-d commented 1 year ago

Makes sense.

When I think about it, leaving them as is makes it a smaller churn (no need to change the workflow files, etc.). I'll create a PR with the addition of one group command, and the explanations of what each custom script command does 👍🏼