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

eslint: Add @wordpress/eslint-plugin/i18n ruleset #49312

Merged
merged 1 commit into from Feb 18, 2021

Conversation

yuliyan
Copy link
Member

@yuliyan yuliyan commented Jan 26, 2021

Changes proposed in this Pull Request

  • Add @wordpress/eslint-plugin/i18n ruleset to root eslint config
  • Disable @wordpress/i18n-text-domain rule, because we use __i18n_text_domain__ global constant in packages instead of string literals.

Testing instructions

  • Checkout the branch locally and yarn install
  • Run yarn eslint on a file that is known not to fulfil the rules from @wordpress/eslint-plugin/i18n and confirm it's reporting the lint errors, e.g. packages/domain-picker/src/domain-picker/suggestion-item.tsx

@yuliyan yuliyan requested review from simison, akirk and a team January 26, 2021 16:12
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 26, 2021
@matticbot
Copy link
Contributor

@scinos
Copy link
Contributor

scinos commented Jan 26, 2021

I'm re-running Check code style for branches (Calypso) with the option to lint all the files, so we can compare with trunk.

@matticbot
Copy link
Contributor

matticbot commented Jan 26, 2021

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@akirk
Copy link
Member

akirk commented Jan 26, 2021

Looks like this revealed quite a number of errors:
Screenshot 2021-01-26 at 20 33 37

Screenshot 2021-01-26 at 20 33 26

@simison
Copy link
Member

simison commented Jan 27, 2021

I'll ping the teams potentially to help here:

  • @Automattic/shilling: client/my-sites/checkout/*
  • @Automattic/cylon : apps/editing-toolkit-plugin/dotcom-fse/* & apps/editing-toolkit-plugin/global-styles/*
  • @Automattic/luna : client/landing/gutenboarding/*, packages/plans-grid/*, packages/domain-picker/*, client/components/language-picker/*

@sirbrillig
Copy link
Member

Thanks for the ping! Should we fix the reported issues in this PR or is the intent that we just be aware of them? All the ones in checkout are @wordpress/i18n-translator-comments, so I've made a PR for them here: #49372

@ciampo
Copy link
Contributor

ciampo commented Jan 28, 2021

@Automattic/luna : client/landing/gutenboarding/*, packages/plans-grid/*, packages/domain-picker/*, client/components/language-picker/*

We're actively editing most of these files in currently open PRs — what is not fixed there, we can add to this PR (or create a separate PR, whatever works best)

@yuliyan
Copy link
Member Author

yuliyan commented Jan 28, 2021

I think fixing them in a separate PR, similarly to #49372, would be better. When the fixes land on trunk, I'll rebase this PR to verify lint errors are gone.

@ciampo
Copy link
Contributor

ciampo commented Jan 28, 2021

I think fixing them in a separate PR, similarly to #49372, would be better. When the fixes land on trunk, I'll rebase this PR to verify lint errors are gone.

Opened #49400 — will let you know once it's approved and merged

@ciampo
Copy link
Contributor

ciampo commented Jan 28, 2021

@yuliyan #49400 has been merged — All errors that were "assigned" to team Luna should be solved 🤞

@yuliyan yuliyan force-pushed the add/wordpress-eslint-plugin-i18n-ruleset branch from bed879a to 9ff7e51 Compare February 18, 2021 09:02
@yuliyan
Copy link
Member Author

yuliyan commented Feb 18, 2021

All of the existing lint problems have been resolved with #49372, #49400, #49526, #50190, #50191. The PR is ready to be merged.

@yuliyan yuliyan merged commit 8f33fe4 into trunk Feb 18, 2021
@yuliyan yuliyan deleted the add/wordpress-eslint-plugin-i18n-ruleset branch February 18, 2021 09:39
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 18, 2021
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.

None yet

7 participants