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

Formats json and yml files using npm run format #1605

Open
wants to merge 12 commits into
base: trunk
Choose a base branch
from
Open
33 changes: 9 additions & 24 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -1,39 +1,24 @@
# This file is for unifying the coding style for different editors and IDEs
# editorconfig.org

# WordPress Coding Standards
# https://make.wordpress.org/core/handbook/best-practices/coding-standards/
# https://developer.wordpress.org/coding-standards/wordpress-coding-standards/

root = true

[*]
charset = utf-8
end_of_line = lf
indent_size = 4
tab_width = 4
indent_style = tab
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = tab

[*.md]
trim_trailing_whitespace = false
[*.yml]
indent_style = space
indent_size = 2

[*.txt]
[*.md]
trim_trailing_whitespace = false

[*.json]
indent_style = space
indent_size = 2

[.*rc]
insert_final_newline = false
indent_style = space
indent_size = 2

[*.yml]
insert_final_newline = false
quote_type = single
indent_style = space
indent_size = 2

[.github/CODEOWNERS]
indent_style = space
[*.txt]
end_of_line = crlf
18 changes: 9 additions & 9 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,26 @@ updates:
interval: weekly
open-pull-requests-limit: 10
labels:
- "no milestone"
- "[Type] Enhancement"
- "github_actions"
- 'no milestone'
- '[Type] Enhancement'
- 'github_actions'

- package-ecosystem: npm
directory: '/'
schedule:
interval: weekly
open-pull-requests-limit: 10
labels:
- "no milestone"
- "[Type] Enhancement"
- "javascript"
- 'no milestone'
- '[Type] Enhancement'
- 'javascript'

- package-ecosystem: composer
directory: '/'
schedule:
interval: weekly
open-pull-requests-limit: 10
labels:
- "no milestone"
- "[Type] Enhancement"
- "php"
- 'no milestone'
- '[Type] Enhancement'
- 'php'
10 changes: 5 additions & 5 deletions .github/workflows/deploy-plugins.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
echo "plugins=$(echo $PLUGINS | jq -c .)" >> $GITHUB_OUTPUT

deploy:
name: "Deploy Plugin: ${{ matrix.plugin }}"
name: 'Deploy Plugin: ${{ matrix.plugin }}'
needs: pre-run
runs-on: ubuntu-latest
permissions:
Expand Down Expand Up @@ -131,7 +131,7 @@ jobs:
with:
step: start
token: ${{ secrets.GITHUB_TOKEN }}
env: "wp.org plugin: ${{ matrix.plugin }}"
env: 'wp.org plugin: ${{ matrix.plugin }}'

- name: Deploy Plugin - ${{ matrix.plugin }}
if: steps.check-deployment.outputs.deploy == 'true'
Expand All @@ -154,12 +154,12 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
status: ${{ job.status }}
deployment_id: ${{ steps.wporg-deployment.outputs.deployment_id }}
env: "wp.org plugin: ${{ matrix.plugin }}"
env_url: "https://wordpress.org/plugins/${{ matrix.plugin }}/"
env: 'wp.org plugin: ${{ matrix.plugin }}'
env_url: 'https://wordpress.org/plugins/${{ matrix.plugin }}/'

release-assets:
name: Add release assets
needs: [ pre-run, deploy ]
needs: [pre-run, deploy]
runs-on: ubuntu-latest
permissions:
actions: read
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/php-test-plugins.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ on:

jobs:
php-test-plugins:
name: "PHP ${{ matrix.php }} / WP ${{ matrix.wp }}"
name: 'PHP ${{ matrix.php }} / WP ${{ matrix.wp }}'
runs-on: ubuntu-latest
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
php: ['8.2', '8.1', '8.0', '7.4', '7.3', '7.2']
wp: [ 'latest' ]
wp: ['latest']
include:
- php: '7.4'
wp: '6.5'
Expand Down
14 changes: 14 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Files and folders related to build
/build
/node_modules
/vendor
/dist

# Minified files
/*.min.js

# Ignore Composer lock file
composer.lock

# Ignore npm package lock file
package-lock.json
1 change: 0 additions & 1 deletion .prettierrc

This file was deleted.

16 changes: 16 additions & 0 deletions .prettierrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Import the default config file and expose it in the project root.
// Useful for editor integrations.
const wpPrettierConfig = require( '@wordpress/prettier-config' );

module.exports = {
...wpPrettierConfig,
overrides: [
{
files: '*.yml',
options: {
useTabs: false,
tabWidth: 2,
},
},
],
};
Comment on lines +1 to +16
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this override?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@swissspidy When both .editorconfig and .prettierrc are present, Prettier gives precedence to .prettierrc. Further the recommended indentation for YAML files is two spaces per level, so we override it here.

Copy link
Member

Choose a reason for hiding this comment

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

But the default Prettier config should be correct for our needs, and the editorconfig should match the Prettier config 🤔

44 changes: 22 additions & 22 deletions .wp-env.json
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
{
"core": null,
"plugins": [
"./plugins/optimization-detective",
"./plugins/auto-sizes",
"./plugins/dominant-color-images",
"./plugins/embed-optimizer",
"./plugins/image-prioritizer",
"./plugins/performance-lab",
"./plugins/speculation-rules",
"./plugins/web-worker-offloading",
"./plugins/webp-uploads"
],
"env": {
"tests": {
"config": {
"FS_METHOD": "direct"
},
"mappings": {
"/wp-content/plugins/performance": "."
}
}
}
"core": null,
Copy link
Member

Choose a reason for hiding this comment

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

Per https://github.com/WordPress/performance/blob/trunk/.editorconfig#L23-L25 the json config it should use two space not tab 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

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 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.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Can we then remove this .prettierrc.js override for yml files? We should be able to use the default config for those too, no?

"plugins": [
"./plugins/optimization-detective",
"./plugins/auto-sizes",
"./plugins/dominant-color-images",
"./plugins/embed-optimizer",
"./plugins/image-prioritizer",
"./plugins/performance-lab",
"./plugins/speculation-rules",
"./plugins/web-worker-offloading",
"./plugins/webp-uploads"
],
"env": {
"tests": {
"config": {
"FS_METHOD": "direct"
},
"mappings": {
"/wp-content/plugins/performance": "."
}
}
}
}
Loading