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

A few minor improvements #75

Closed
wants to merge 6 commits into from
Closed

A few minor improvements #75

wants to merge 6 commits into from

Conversation

olalonde
Copy link

@olalonde olalonde commented Aug 6, 2015

Inventory.prototype.typeName method

Add optional callback to Peer.prototype.sendMessage

Get rid of deprecation warning caused by node-buffers library.

@olalonde olalonde changed the title Add typeName() utility method to Inventory A few minor improvements Aug 7, 2015
@@ -54,7 +54,7 @@
"dependencies": {
"bitcore": "^0.12.0",
"bloom-filter": "^0.2.0",
"buffers": "^0.1.1",
"buffers": "olalonde/node-buffers",
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to node-buffers look good, however this references an unpublished master branch. There are a few open issues, #52 and #32 with node-buffers already. I believe we only use a small portion of the implemented methods, so we may be able to make this a part of this codebase, since we're also adding the skip method. However we can take care of this in another PR as well as the deprecation warnings.

@olalonde
Copy link
Author

olalonde commented Aug 9, 2015

Thanks for the comments, will update the PR tomorrow.

@koalalorenzo
Copy link

Any update? Will this ever be approved and merged?

@braydonf
Copy link
Contributor

Needs a rebase on master. It would be good to fix the issues related to buffers in a released version instead of pointing to master branch. Preferably we could bring in what we need from buffers into this project, since there are only a few methods we're using, this could be handled in a separate PR.

@olalonde olalonde closed this Feb 19, 2023
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