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

Various eslint-suggested fixes #162

Closed
4 tasks done
lhstrh opened this issue Jun 5, 2023 · 0 comments · Fixed by #175
Closed
4 tasks done

Various eslint-suggested fixes #162

lhstrh opened this issue Jun 5, 2023 · 0 comments · Fixed by #175

Comments

@lhstrh
Copy link
Member

lhstrh commented Jun 5, 2023

          Yes and no - I made sure `eslint` won't complain, but some are simply suppressed by `eslint-disable-next-line` and added as TODO items.

Here are some examples:

  • @typescript-eslint/no-this-alias is suppressed entirely; it is addressed in Possibilities of eliminating binding workarounds #159
  • Some Java style code that does not live well with ts, for example, empty abstract class and classes with only static methods
  • interface IOPortManager<T extends Present> extends TriggerManager {
    // TODO (axmmisaka): Function property is better in terms of type checking
    // as tsc will check additional info; yet that additional check will cause massive issues
    // and therefore this linter rule is disabled and method signature is used.
    // However, this indicates that there are typing issues and needs to be addressed.
    // eslint-disable-next-line @typescript-eslint/method-signature-style
    addReceiver(port: WritablePort<T>): void;
    // eslint-disable-next-line @typescript-eslint/method-signature-style
    delReceiver(port: WritablePort<T>): void;
    }
    ; if this is changed the whole reactor.ts will explode
  • We have a lot of use of any and maybe there could be alternatives (mostly solved by Improved typing #172 and Remove Present and replace with unknown; remove Args and Trigger and use TS Tuple #168)

Yet I think these changes will involve complex refactoring and should be addressed in individual issues

Originally posted by @axmmisaka in #160 (comment)

@axmmisaka axmmisaka changed the title Various es-lint-suggested fixes Various eslint-suggested fixes Jun 7, 2023
@lhstrh lhstrh mentioned this issue Jul 2, 2023
4 tasks
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 a pull request may close this issue.

1 participant