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

ci(eslint): add eslint-plugin-xdependency #52

Merged

Conversation

tiborux
Copy link
Contributor

@tiborux tiborux commented Jul 22, 2021

Add eslint-plugin-x

@tiborux tiborux requested a review from a team as a code owner July 22, 2021 07:18
@CachedaCodes CachedaCodes self-requested a review July 22, 2021 08:56
Copy link
Contributor

@CachedaCodes CachedaCodes left a comment

Choose a reason for hiding this comment

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

Should we add a .eslintignore with

**/*.js
types

Like the other packages?

@davidmfempathy davidmfempathy self-requested a review July 22, 2021 10:56
{
"extends": ["plugin:@empathyco/x/all"],
"rules": {
"@typescript-eslint/restrict-template-expressions": ["off", { "allowAny": true }],
Copy link
Contributor

Choose a reason for hiding this comment

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

I've noticed that both problems come from: x/packages/jest-utils/src/jest-utils.ts.
Isn't it preferable to disable both rules at least just for that file ?

@@ -0,0 +1,4 @@
{
"extends": "./tsconfig.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just have everything in the tsconfig.eslint.json instead of extending it from another file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is how its done in other packages

@@ -1,22 +1,23 @@
export interface EmpathyExtendedExpect {
arrayOf(classType: Newable): any;
arrayOfItemsMatching(schema: {}): any;
arrayOfItemsMatching(schema: Record<string, any>): any;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my information, did the linter complain about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the linter doesn't like to use an empty object, so i added a record

@tiborux
Copy link
Contributor Author

tiborux commented Jul 22, 2021

@CachedaCodes we dont have types and js files in this package

@davidmfempathy davidmfempathy merged commit 080aa2c into main Jul 23, 2021
@tajespasarela tajespasarela deleted the feature/EX-4317-add-eslint-plugin-x-to-x-jest-ut_ branch July 27, 2021 06:16
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.

3 participants