-
Notifications
You must be signed in to change notification settings - Fork 272
Conversation
0734ef7
to
d1fc502
Compare
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.
Generally looks good, two things (+ 1 typo) to address.
src/bytes.ts
Outdated
*/ | ||
export const setLengthLeft = function(msg: any, length: number, right: boolean = false) { | ||
export const setLengthLeft = function(msg: Buffer, length: number, right: boolean = false) { |
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.
Hmm, this is pretty confusing to have this boolean right
parameter in the API. Can we instead do an internal setLength
method and move the logic over there and do function signature (so: function(msg: Buffer, length: number)
) and internal structure analogue to setLengthRight
?
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.
Yes, that's definitely nicer.
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.
Addressed in a2a50ae
src/bytes.ts
Outdated
* @param a (Buffer) | ||
* @return (Buffer) | ||
*/ | ||
export const unpadBuffer = function(a: any): Buffer { |
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 assume having an any
type here is a mistake?
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.
Ai! Thanks.
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.
Addressed in 1f2803c
src/bytes.ts
Outdated
} | ||
|
||
/** | ||
* Trims leading zeros from a `Array` (of numbers). |
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.
Nit-pick "an Array
"
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.
Addressed in 82ab4b4
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.
Addressed in 82ab4b4
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.
Looks good
Part of #172 Breaking API changes
(Will make similar changes for the methods in hash.ts and signature.ts in a separate PR)