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

Formats json and yml files using npm run format #1605

Open
wants to merge 14 commits into
base: trunk
Choose a base branch
from
Open

Formats json and yml files using npm run format #1605

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f101786
Formats json and yml files using npm run format
devansh016 Oct 18, 2024
3fed57a
Adds override config for yml and json
devansh016 Oct 18, 2024
eaaa59d
Formats json and yml files
devansh016 Oct 18, 2024
a3ae0e1
Add singleQuote for yml files
devansh016 Oct 18, 2024
d336d2a
Ignores generated lock files
devansh016 Oct 18, 2024
90ac758
Updates .editorconfig as per core and Gutenberg
devansh016 Oct 21, 2024
1d13a44
Formats using npm run format-js
devansh016 Oct 21, 2024
a565773
Updates patterns to ignore
devansh016 Oct 21, 2024
afc3339
Overides .yml config to use 2 spaces
devansh016 Oct 21, 2024
4a9d135
Adds file end line and update WordPress coding standards URL
devansh016 Oct 21, 2024
f9a6838
Merge branch 'trunk' of github.com:devansh016/wp-performance into For…
devansh016 Oct 28, 2024
6773f8e
Merge branch 'Format-files-using-npm-run-format-js' of github.com:dev…
devansh016 Oct 28, 2024
376b0fd
Merge branch 'trunk' of github.com:devansh016/wp-performance into For…
devansh016 Nov 19, 2024
9356e5a
Removes yml file override from prettier config
devansh016 Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 9 additions & 24 deletions .editorconfig
View file
Open in desktop
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://make.wordpress.org/core/handbook/coding-standards/
devansh016 marked this conversation as resolved.
Show resolved Hide resolved

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
devansh016 marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 9 additions & 9 deletions .github/dependabot.yml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

16 changes: 16 additions & 0 deletions .prettierrc.js
View file
Open in desktop
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,
},
},
],
};
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 🤔

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 The default Prettier configuration should suffice for our needs, and ensuring that the .editorconfig aligns with it eliminates the need for overrides. I have remove this overrides and now the .yml files formating looks similar to one in gutenberg project ref.

44 changes: 22 additions & 22 deletions .wp-env.json
View file
Open in desktop
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
Loading