-
Notifications
You must be signed in to change notification settings - Fork 22
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: put processes in class for state #781
Conversation
Big change :D what's the pros/cons? |
Having more and more "floating" state was bothering me + passing all params for each function starts to be too verbose. |
Note: it's easier to review with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code itself lgtm! would like to see if there's a way where the eslint rule doesn't need to be disabled
const { seq } = await this.stateManager.get(); | ||
const changesConsumer = this.changesConsumer; | ||
if (!changesConsumer) { | ||
log.warn('Can not watch without a consumer'); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we return undefined
, is the eslint rule disabling still needed?
return; | |
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue for that. we could disable the rule entirely for TS though
typescript-eslint/typescript-eslint#1277
## [1.4.10](v1.4.9...v1.4.10) (2021-11-04) ### Bug Fixes * put processes in class for state sake ([#781](#781)) ([275231c](275231c))
No description provided.