types: More type defintions #178
Conversation
| decode(buf: Buffer, pos?: number, resolver?: any): { value: any, offset: number}; | ||
| encode(val: any, buf: Buffer, pos?: number): void; | ||
| equals(type: Type): boolean; | ||
| fingerprint(algorithm?: string): Buffer | string; |
There was a problem hiding this comment.
As far as I can see fingerprint can return Buffers only
There was a problem hiding this comment.
I think so too. I was deferring to https://nodejs.org/api/crypto.html#crypto_class_hash => https://nodejs.org/api/stream.html#stream_readable_read_size
but more than happy to change
| decode(buf: Buffer, pos?: number, resolver?: any): any; | ||
| encode(val: any, buf: Buffer, pos?: number): void; | ||
| fingerprint(algorithm?: any): any; | ||
| decode(buf: Buffer, pos?: number, resolver?: any): { value: any, offset: number}; |
There was a problem hiding this comment.
Decode: Good catch, I added that to my pullrequest as well, hoping all of this gets committed faster to the project.
There was a problem hiding this comment.
Feel free to pull anything in! I didn't even realize there was already a PR out with a lot of the changes -- nice work -- I've sort of slowly PR-ing bits as I've been coming across them.
I'll let @mtth merge as he see fits. (I prefer small uncontroversial PRs, so that there's higher velocity) but I'm more than happy to see these subset of changes get merged with your PR if everything checks out.
Update based on pull request mtth/avsc mtth#178
|
@alexander-alvarez - are there still changes from this PR you wanted to merge or is this covered by #175? |
|
nope they're incorporated 👍 |
toJSONwas actually wrong, which is worse than too generic types (i.e. any).Type.equalsmethod wasn't defined in the typings file