Scripts: Include YAML files in prettification #30240
Conversation
ockham
added
[Type] Build Tooling
|
Size Change: +31.1 kB (+2%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
|
Interesting, so it also forces 4 spaces for indentation? |
|
In general, I love the idea. I'd like to see all file formats covered one day. At least for Gutenberg.
It would be simpler to override some settings if JS wouldn't be concerned in the mix. As noted in the comment, we probably should create its own group that is concerned with files used as configs like YAML or JSON. Any thoughts on how we can ensure consistent formatting with CI? |
Yeah, I think it basically just applies the same gutenberg/packages/prettier-config/lib/index.js Lines 14 to 15 in 2cdc0f6 Personally, I'm fine with whatever, as long as it's auto-formatted 😬 and I like to keep a minimum of config files. |
Agreed, it's simpler to keep one set of rules for everything. In case we want some tweaks, you can override settings with CLI flags. |
ockham
force-pushed
the
add/yaml-formatting
branch
from
6eaa8b5
to
09dee95
Compare
|
This should be ready for another look: I've now renamed |
I tried adding JSON, but I'd rather do it in a separate PR for the following reasons:
In conclusion, I'm worried more about scope creep for this now still fairly-contained PR more than about adding support for JSON in a separate step (especially after we've renamed Are there any other file formats you had in mind? For reference, I've found a list of supported formats here. Diff for enabling JSONdiff --git a/package.json b/package.json
index 99d31594a5..c8aeb17891 100644
--- a/package.json
+++ b/package.json
@@ -292,7 +292,7 @@
"*.scss": [
"wp-scripts lint-style"
],
- "*.{js,ts,tsx,yml}": [
+ "*.{js,json,ts,tsx,yml}": [
"wp-scripts format",
"wp-scripts lint-js"
],
diff --git a/packages/scripts/CHANGELOG.md b/packages/scripts/CHANGELOG.md
index b31a5c8ab2..0fb7a81378 100644
--- a/packages/scripts/CHANGELOG.md
+++ b/packages/scripts/CHANGELOG.md
@@ -5,7 +5,7 @@
### Breaking Changes
- Rename `format-js` script to `format` ([#30240](https://github.com/WordPress/gutenberg/pull/30240)).
-- Include YAML files when formatting files with `format` ([#30240](https://github.com/WordPress/gutenberg/pull/30240)).
+- Include JSON and YAML files when formatting files with `format` ([#30240](https://github.com/WordPress/gutenberg/pull/30240)).
- The bundled `css-loader` dependency has been updated from requiring `^3.5.2` to requiring `^5.1.3` ([#27821](https://github.com/WordPress/gutenberg/pull/27821)).
- The bundled `mini-css-extract-plugin` dependency has been updated from requiring `^0.9.0` to requiring `^1.3.9` ([#27821](https://github.com/WordPress/gutenberg/pull/27821)).
- The bundled `postcss-loader` dependency has been updated from requiring `^3.0.0` to requiring `^4.2.0` ([#27821](https://github.com/WordPress/gutenberg/pull/27821)).
diff --git a/packages/scripts/scripts/format.js b/packages/scripts/scripts/format.js
index 6dd655bdab..40547216e1 100644
--- a/packages/scripts/scripts/format.js
+++ b/packages/scripts/scripts/format.js
@@ -104,7 +104,7 @@ if ( fileArgs.length === 0 ) {
// Converts `foo/bar` directory to `foo/bar/**/*.js`
const globArgs = dirGlob( fileArgs, {
- extensions: [ 'js', 'jsx', 'ts', 'tsx', 'yml', 'yaml' ],
+ extensions: [ 'js', 'json', 'jsx', 'ts', 'tsx', 'yml', 'yaml' ],
} );
const result = spawn( |
ockham
force-pushed
the
add/yaml-formatting
branch
from
01e75fd
to
10050ae
Compare
There are ways to mitigate it. You can always apply formatting in separate PRs, one for each file extension type before we introduce more strict integration. I guess that's the only concern in terms of the size of this PR.
We also use JSON and SCSS from the list of supported types of files. |
Well, there goes my plan for a quick PR to get YAML formatted, and iterate later on it to include other formats in order to keep the cognitive load smaller 😬
If we really want to inflate the scope of this to include different formats that currently have their own linters etc, we might want to consider following a similar strategy as @scinos mentioned:
|
ockham
force-pushed
the
add/yaml-formatting
branch
from
10050ae
to
69d6230
Compare
This seems to be the relevant Calypso PR: Automattic/wp-calypso#40118 |
Hmm, |
@ntwb tried that approach in the past and it didn't work very well. Related PR and the conclusions can be found in #4628 (comment) and #4628 (comment). I can't find the exact comment, maybe it's another PR but there were some issues with |
|
One more thing, we will need to add missing handling for gutenberg/packages/scripts/scripts/format-js.js Lines 83 to 91 in 719d575 We can copy gutenberg/packages/scripts/config/.eslintignore Lines 1 to 3 in 719d575
|
|
I opened PRs that try to format: |
ockham
force-pushed
the
add/yaml-formatting
branch
from
69d6230
to
e6b10b1
Compare
|
Two more comments to address and we can land it:
If you prefer to tackle ignored files separately, then we can split it, too. |
Co-authored-by: Greg Ziółkowski <[email protected]>
|
Thanks for approving, @gziolo! ❤️
I'll file a separate PR for this. I tried it locally, and it surprisingly changes the formatting of some files in the
✅ I will include some more indentation changes to |
`prettier` has been able to [format YAML files since version 1.14](https://prettier.io/blog/2018/07/29/1.14.0.html) (released in July of 2018). We're starting to size up on YAML files (mostly GitHub Actions workflows), so it'd be nice to have them auto-formatted. This PR adds YAML files to the list of extensions formatted by `prettier`. Existing YAML files have already been updated to the new format by #30409. Co-authored-by: Greg Ziółkowski <[email protected]> When backporting, `format-js` remains exactly the same, and only the `.yml` extension is added to the `format-js` command in `package.json`. (cherry picked from commit 06fd22e)
Description
prettierhas been able to format YAML files since version 1.14 (released in July of 2018). We're starting to size up on YAML files (mostly GitHub Actions workflows), so it'd be nice to have them auto-formatted. This PR adds YAML files to the list of extensions formatted byprettier. Existing YAML files have already been updated to the new format by #30409.How has this been tested?
Verify that GH Actions still pass.
Types of changes
Developer experience
cc/ @jsnajdr @scinos @tyxla @anomiex @jeherve In case y'all wanna do something similar for Calypso and Jetpack