refactor: Use options pattern in RedisPool constructor #468
Conversation
|
Sorry for the late review, @jcw-. Thanks for opening this PR 👍 |
|
No worries at all, thanks for the suggestion! |
| this.redisPool = pool; | ||
| } | ||
|
|
||
| async getClient() { | ||
| return await this.redisPool.getClient(); | ||
| return this.redisPool.getClient(); |
There was a problem hiding this comment.
Just out of interest, does removing the await change the semantics of getClient()? It seems like it now returns a promise that needs to be await-ed, whereas before the result of the promise was returned.
There was a problem hiding this comment.
@hassankhan await required if you have try/catch/finally block inside a method. Otherwise it works the same way.
There was a problem hiding this comment.
FYI, it was the linting that led me to remove that, this rule violation was triggered (not sure why I didn't notice it originally): https://eslint.org/docs/rules/no-return-await
|
@jcw- Looks great! Thanks for providing it! |
Check List
Description of Changes Made (if issue reference is not provided)
Instead of passing in values directly through the constructor, use the options pattern for easier extensibility.
From a review of #433