-
Notifications
You must be signed in to change notification settings - Fork 534
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
redis cleanup #195
redis cleanup #195
Conversation
Codecov Report
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
- Coverage 95.29% 95.22% -0.08%
==========================================
Files 88 93 +5
Lines 4381 4750 +369
Branches 478 492 +14
==========================================
+ Hits 4175 4523 +348
- Misses 206 227 +21
|
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.
I would prefer to just add type
for all imports
import type * as redisTypes from 'redis'
without really changing redisTypes into redis
and be consistent everywhere so avoid things like
import type { Callback } from 'redis';
but rather have
import type * as redisTypes from 'redis'
// and then
callback: redisTypes.Callback<unknown>;
It is much more intuitive to see something like redisTypes.Callback
instead of Callback
or later
redisTypes.RedisClient
instead of RedisClient
mixed with RedisPluginClientTypes
and trying to figure out which type comes from where
…d type where applicable Signed-off-by: Naseem <[email protected]>
Signed-off-by: Naseem <[email protected]>
c3bfba8
to
806663b
Compare
Alright I've adjusted as requested. |
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, thx
Which problem is this PR solving?
Short description of the changes
RedisPluginStreamTypes