search

near / borsh-js

TypeScript/JavaScript implementation of Binary Object Representation Serializer for Hashing
Apache License 2.0
112 stars 38 forks source link

Add u16, u256, and u512 for ser/de with test #11

Closed joncinque closed 3 years ago

joncinque commented 3 years ago

This expands the different possibilities for large unsigned numbers, including u256 and u512. Also, the implementation for u16 was missing, so add that in as well!

The u256 and u512 are particularly useful for public keys represented as a BN in front-end code.

joncinque commented 3 years ago

Let me know what I need to do for the CI -- it seems like there's a missing FOSSA_API_KEY

ailisp commented 3 years ago

@volovyk-s @chefsale can we figure out run CI for external contributors like borsh-rs, at least we run some of tests, if there's security concern

chefsale commented 3 years ago

@ailisp the problem is that the env variables are not currently applied on the forks and this makes the checks to fail. I will be taking a look on how to make this work.

chefsale commented 3 years ago

I think for now we could add a check in travis.yml which skips the fossa integration on forks and than runs it on master eitherway. cc: @ailisp what do you think, this would be a temp solution which isn't optimal, it is non trivial to make this work properly.

joncinque commented 3 years ago

What's your approval and release procedure typically? I'd like to get a sense of when I might be able to integrate these changes in my own code

joncinque commented 3 years ago

@ailisp Is there anything else required here? I would like to start using it if possible!

ailisp commented 3 years ago

@joncinque sorry for the delay. I've merged the other PR from you. For this one, I create #15 to trigger ci and will merge if pass. Thank you!

ailisp commented 3 years ago

@joncinque I merged #15 which includes your commits and resolved conflict, so will close this

ailisp commented 3 years ago

@chefsale Sorry somehow I missed these replies. Looks like foss also run tests, can we have some basic test for fork (just npm run test), and foss only on master. For example this build was also on a fork and it works: https://travis-ci.com/github/near/borsh-js/jobs/473781317/config

joncinque commented 3 years ago

@ailisp Thanks so much! In the future, do you have a preferred way to communicate? At Solana, we've started leaning heavily on all of the great Borsh libraries, so we're happy to get in touch however you prefer :smile:

ailisp commented 3 years ago

@joncinque Hi Jon, glad to connect! I watch discord frequently. I'm also in solana discord server and my handle is Bo | NEAR#8394