-
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: enable eslint @typescript-eslint/no-floating-promises rule #245
Conversation
Signed-off-by: Denis Golovin <[email protected]>
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.
fix in source code will be part of the PR ?
as we can't merge as it is not green using the PR check
Of course it should be part of the PR, I wanted to see the scope of the problem before fixing all the errors. |
Signed-off-by: Denis Golovin <[email protected]>
Signed-off-by: Denis Golovin <[email protected]>
@dgolovin I would love to see this PR to be part of next release. I think that some problems might be avoided with awaiting for asynchronous functions to finish. |
@odockal If it is approved, then I can merge it. |
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.
The void pattern on promise is to be replaced by await as the flow may be strange to the user
Signed-off-by: Denis Golovin <[email protected]>
Signed-off-by: Denis Golovin <[email protected]>
@jeffmaury I added |
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.
LGTM
Related to #244.
Indeed there are 29 floating promises