Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace node buffers with uint8arrays #730

Merged
merged 10 commits into from
Aug 24, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Aug 12, 2020

Upgrades all deps and replaces all use of node Buffers with Uint8Arrays

Depends on:

BREAKING CHANGES:

  • All deps used by this module now use Uint8Arrays in place of node Buffers

@achingbrain achingbrain requested review from jacobheun and vasco-santos and removed request for jacobheun August 12, 2020 16:27
@achingbrain achingbrain requested a review from jacobheun August 12, 2020 16:33
@achingbrain
Copy link
Member Author

This is using a temporary fork of libp2p-noise that has the changes from ChainSafe/js-libp2p-noise#70 applied and the typescript compiled to js.

@jacobheun
Copy link
Contributor

Looks like the browser tests are failing

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good in a first glimpse!
However, one test is being specified to run by itself

test/keychain/cms-interop.spec.js Outdated Show resolved Hide resolved
@jacobheun
Copy link
Contributor

I am going to change the base of this to the 0.29.x branch so I can update the new signed peer records work. I'd like to land these together in 0.29

@jacobheun jacobheun changed the base branch from master to 0.29.x August 21, 2020 14:58
achingbrain and others added 7 commits August 21, 2020 17:01
@jacobheun jacobheun force-pushed the fix/replace-node-buffers-with-uint8arrays branch from fbeecde to 2143508 Compare August 21, 2020 15:02
@jacobheun
Copy link
Contributor

With libp2p/js-libp2p-interfaces#62 this should be good to go for the 0.29 branch.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
pending the interface release

@jacobheun
Copy link
Contributor

Ci is failing due to interop, this is a release order issue with the daemon. We can resolve this before release.

@jacobheun jacobheun merged commit 507f8c4 into 0.29.x Aug 24, 2020
@jacobheun jacobheun deleted the fix/replace-node-buffers-with-uint8arrays branch August 24, 2020 10:58
jacobheun added a commit that referenced this pull request Aug 27, 2020
* fix: replace node buffers with uint8arrays

Upgrades all deps and replaces all use of node Buffers with Uint8Arrays

BREAKING CHANGES:

- All deps used by this module now use Uint8Arrays in place of node Buffers

* chore: browser fixes

* chore: remove .only

* chore: stringify uint8array before parsing

* chore: update interop suite

* chore: remove ts from build command

* chore: update deps

* fix: update records to use uint8array

* chore: fix lint

* chore: update deps

Co-authored-by: Jacob Heun <jacobheun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants