New RESP2 parser #1899
New RESP2 parser #1899
Conversation
|
This pull request introduces 2 alerts when merging b0b2464 into 9f0f7f5 - view on LGTM.com new alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #1899 +/- ##
==========================================
+ Coverage 95.37% 95.46% +0.08%
==========================================
Files 392 396 +4
Lines 3546 3682 +136
Branches 417 442 +25
==========================================
+ Hits 3382 3515 +133
- Misses 86 87 +1
- Partials 78 80 +2
Continue to review full report at Codecov.
|
|
|
||
| it('should compose two buffers', () => { | ||
| composer.write(Buffer.from([0])); | ||
| assert.deepEqual( |
There was a problem hiding this comment.
Question: Are deepequal comparisons expensive in node? They are in Go.
| #findCRLF(chunk: Buffer): number { | ||
| for (let i = this.#cursor; i < chunk.length; i++) { | ||
| if (chunk[i] === CR) { | ||
| return i; |
There was a problem hiding this comment.
Might be a style difference. In python I'm more likely to do something like:
f = -1
for thing ... <my loop>:
if things match:
f = thing
break
return f
I find it makes it easier to read functions when you're not looking for multiple returns
|
|
||
| #arrayItemType?: Types; | ||
|
|
||
| #parseArray(chunk: Buffer, arraysToKeep = 0): ArrayReply | undefined { |
There was a problem hiding this comment.
Yes, this feels ugly - you're right.
It also looks right.
|
This pull request introduces 1 alert when merging 1331698 into 11b0c06 - view on LGTM.com new alerts:
|
| function generateTests({ | ||
| toWrite, | ||
| returnStringsAsBuffers, | ||
| replies | ||
| }: TestsOptions): void { |
packages/client/lib/client/socket.ts
Outdated
| } catch (err) { | ||
| this.#socket.destroy(); | ||
| this.#socket = undefined; | ||
|
|
||
| throw err; | ||
| if (err instanceof AuthError) { | ||
| this.#isOpen = false; | ||
| } | ||
|
|
||
| if (!this.#isOpen) return; | ||
| throw err; | ||
| } |
No description provided.