Formats json and yml files using npm run format #1605
Conversation
westonruter
added
no milestone
devansh016
requested review from
mukeshpanchal27,
felixarntz and
thelovekesh
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Let's also do linting of JSON and YML files in the lint-staged config so this doesn't happen again. Also, why isn't the formatting not causing a failure on the GHA checks? |
|
Will Dependabot updates undo the changes to |
| } | ||
| } | ||
| } | ||
| "core": null, |
There was a problem hiding this comment.
Per https://github.com/WordPress/performance/blob/trunk/.editorconfig#L23-L25 the json config it should use two space not tab 🤔
There was a problem hiding this comment.
It seems that when running npm run format-js, the .editorconfig settings are not being applied, which is causing JSON files to use tabs instead of 2 spaces as specified in the .editorconfig. It appears the command is currently using the .prettierrc configuration instead.
There was a problem hiding this comment.
When both .editorconfig and .prettierrc are present, Prettier gives precedence to .prettierrc. You can override config for yml and json to change this behavior from the Prettier config file - https://prettier.io/docs/en/configuration.html#configuration-overrides
There was a problem hiding this comment.
Seems strange that we have to apply a manual override for the default Prettier configuration used by wp-scripts, as that is the de facto standard.
Our .editorconfig file looks quite different from what's in core or Gutenberg. They both use tabs for JSON files for example.
So I think we should rather update our .editorconfig rather than the Prettier config.
There was a problem hiding this comment.
Yeah, I noticed that too the other week. FWIW the .editorconfig we use is old, and a long time ago I believe this was common in WP land to use two spaces for those kinds of files. But since Gutenberg provides centralized tooling that dictates otherwise, I feel like it would make sense to align with that.
There was a problem hiding this comment.
I have updated the .editorconfig file to align with the settings used in Core and Gutenberg, where JSON files use tabs instead of 2 spaces. This should prevent conflicts with the existing Prettier configuration and ensure consistency across the project.
|
|
I think we squash and merge this one and include an ignore commit for the merge commit to hide it in git blame. Ref: https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view. |
|
We should exclude generated lock files from Prettier, just to avoid conflicts |
Yes, we should ignore any auto-generated files. @devansh016 can you please add a https://prettier.io/docs/en/ignore.html with such files? |
|
See also WordPress/gutenberg#30714 in which Gutenberg started applying formatting to JSON files. |
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
I think we still need to confirm Dependabot's behavior. We wouldn't want it constantly reverting tabs back to spaces. |
Dependabot generally respects the indentation style in |
Summary
Fixes #1603
Relevant technical choices
The following files have been formatted by using
npm run format-jsNote: .yml and .json files were not ignored to ensure they are properly formatted as well.