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

Update eslint-plugin-wpvip to 0.5.8, Prettier updates #1412

Merged
merged 11 commits into from
Jun 23, 2023

Conversation

chriszarate
Copy link
Member

@chriszarate chriszarate commented Jun 20, 2023

Description

  • Update eslint-plugin-wpvip to 0.5.8 which contains additional lint rules and expands Prettier config to work with JSON, Markdown, and YAML files.
  • Install wp-prettier
  • Add .editorconfig
  • Standardize GitHub Action workflows to reduce unnecessary runs and run Prettier check.
  • Standardize npm scripts
  • Update code to fix lint errors
  • Remove disabled rules in favor of ad hoc eslint-disable-next-line
  • Apply Prettier fixes

What should I review?

Since this is a formatting-heavy PR, I recommend reviewing commit-by-commit and ignoring the "Prettier formatting" commits. Code changes to address lint errors deserve attention, as do any changes to package.json, .eslintrc, and any changed ignore files.

It's not particularly helpful to review the formatting changes themselves, since they are automated and well-tested upstream.

Steps to Test

  1. Check out PR.
  2. Run npm run format
  3. Run npm run lint

@chriszarate chriszarate requested a review from sjinks June 20, 2023 22:48
@chriszarate chriszarate changed the title Update eslint-plugin-wpvip to 0.5.8 Update eslint-plugin-wpvip to 0.5.8, Prettier updates Jun 21, 2023
Comment on lines 114 to 159
const result = await cliTest.spawn( [
process.argv[ 0 ],
vipDevEnvCreate,
'--slug', slug,
'--app-code', 'image',
'--title', expectedTitle,
'--multisite', `${ expectedMultisite }`,
'--php', expectedPhpVersion,
'--wordpress', expectedWordPressVersion,
'--mu-plugins', 'image',
'-e', `${ expectedElasticsearch }`,
'-p', `${ expectedPhpMyAdmin }`,
'-x', `${ expectedXDebug }`,
'-A', `${ expectedMailpit }`,
'-H', `${ expectedPhoton }`,
], { env }, true );
const result = await cliTest.spawn(
[
process.argv[ 0 ],
vipDevEnvCreate,
'--slug',
slug,
'--app-code',
'image',
'--title',
expectedTitle,
'--multisite',
`${ expectedMultisite }`,
'--php',
expectedPhpVersion,
'--wordpress',
expectedWordPressVersion,
'--mu-plugins',
'image',
'-e',
`${ expectedElasticsearch }`,
'-p',
`${ expectedPhpMyAdmin }`,
'-x',
`${ expectedXDebug }`,
'-A',
`${ expectedMailpit }`,
'-H',
`${ expectedPhoton }`,
],
{ env },
true
);
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, the original version was more readable :-(

Copy link
Member Author

@chriszarate chriszarate Jun 22, 2023

Choose a reason for hiding this comment

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

I added exemptions for this and similar invocations with // prettier-ignore

@chriszarate chriszarate force-pushed the update/eslint-wpvip-0.5.8 branch from 6252b96 to a998aca Compare June 22, 2023 19:35
@sjinks sjinks merged commit 5cf5e3f into develop Jun 23, 2023
@sjinks sjinks deleted the update/eslint-wpvip-0.5.8 branch June 23, 2023 06:23
@gudmdharalds gudmdharalds restored the update/eslint-wpvip-0.5.8 branch November 20, 2023 17:14
@gudmdharalds gudmdharalds deleted the update/eslint-wpvip-0.5.8 branch November 20, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants