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

[Cases] Add eslintrule to enforce display names and ignore existing problems #123692

Merged
merged 6 commits into from
Jan 27, 2022

Conversation

academo
Copy link
Contributor

@academo academo commented Jan 25, 2022

Summary

This PR is a follow up to #123460

in short:

  • enforces react/display-name with ignoreTranspilerName on all cases plugin files inside the public folder
  • Ignore the existing errors that were not fixed on the previous PR. Most of these errors are anonymous functions that are not named or can't be named automatically.
  • Fixes a missing component that can have a displayName

Note: Better to see this PR disabling whitespaces in github. No business logic was changed in this PR, only adding comments and running eslint --fix on top which changed indentation.

All these eslint ignores were added with https://github.com/amanda-mitchell/suppress-eslint-errors

@academo academo added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.1.0 labels Jan 25, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

@peteharverson peteharverson changed the title [Cases] Add eslintrule to enfoce display names and ignore existing problems [Cases] Add eslintrule to enforce display names and ignore existing problems Jan 25, 2022
@@ -1070,6 +1070,12 @@ module.exports = {
'@typescript-eslint/no-duplicate-imports': ['error'],
},
},
{
files: ['x-pack/plugins/cases/public/**/*.{js,mjs,ts,tsx}'],
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you feel about ignoring test files? I think we could add this: https://eslint.org/docs/user-guide/configuring/ignoring-code#ignorepatterns-in-config-files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonathan-buttner I excluded test files and removed the changes from them.

@academo academo enabled auto-merge (squash) January 26, 2022 15:05
Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Thanks for excluding the test files! Looks good.

@academo
Copy link
Contributor Author

academo commented Jan 26, 2022

@elasticmachine merge upstream0

@jonathan-buttner
Copy link
Contributor

@elasticmachine merge upstream

Verified

This commit was created on github.com and signed with GitHub’s verified signature. The key has expired.
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
cases 70 83 +13

Total ESLint disabled count

id before after diff
cases 89 102 +13

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@academo academo merged commit 3e7611b into elastic:main Jan 27, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants