Extract isDefinition helper
#196
Conversation
| @@ -10,5 +10,7 @@ module.exports = { | |||
| ignorePatterns: ['temp', 'scip.ts', 'snapshots'], | |||
| rules: { | |||
| 'no-sync': 'off', | |||
| 'jsdoc/check-indentation': 'off', | |||
| 'class-methods-use-this': 'error', | |||
There was a problem hiding this comment.
Follow-up on this comment by @varungandhi-src. This rule forbids class methods without instance context usage. @olafurpg, let me know what you think about that!
There was a problem hiding this comment.
This is really nice. Big +1 from me
| @@ -707,3 +681,46 @@ function isEqualArray<T>(a: T[], b: T[]): boolean { | |||
| } | |||
| return true | |||
| } | |||
|
|
|||
| function declarationName(node: ts.Node): ts.Node | undefined { | |||
There was a problem hiding this comment.
Extracted into an independent function to satisfy the class-methods-use-this ESLint rule.
There was a problem hiding this comment.
It's a big improvement in readability to have a toplevel function instead of an instance method.
| @@ -10,5 +10,7 @@ module.exports = { | |||
| ignorePatterns: ['temp', 'scip.ts', 'snapshots'], | |||
| rules: { | |||
| 'no-sync': 'off', | |||
| 'jsdoc/check-indentation': 'off', | |||
| 'class-methods-use-this': 'error', | |||
There was a problem hiding this comment.
This is really nice. Big +1 from me
| @@ -707,3 +681,46 @@ function isEqualArray<T>(a: T[], b: T[]): boolean { | |||
| } | |||
| return true | |||
| } | |||
|
|
|||
| function declarationName(node: ts.Node): ts.Node | undefined { | |||
There was a problem hiding this comment.
It's a big improvement in readability to have a toplevel function instead of an instance method.
This PR improves the readability of the
isDefinitionlogic. Follow-up on this comment.Test plan
Tests are green, and the comment on the
isDefinitionfunction looks ok.