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

Import bufferput into bitcore #75

Closed
ryanxcharles opened this issue Feb 17, 2014 · 6 comments
Closed

Import bufferput into bitcore #75

ryanxcharles opened this issue Feb 17, 2014 · 6 comments

Comments

@ryanxcharles
Copy link
Contributor

We constanty have issues with bufferput. Would be easier to deal with if we just imported it into bitcore.

@discretepackets
Copy link

I think part of the problem is because the bufferput module in the npm registry hasn't been updated with the fix from this commit bitpay/node-bufferput@6b2ad51

(i.e. the bufferput module from npm install bitcoin or from npm install in a cloned bitcore repo doesn't contain the fixed version)

@maraoz
Copy link
Contributor

maraoz commented Feb 18, 2014

What do you think of updating the dependency to point to https://github.com/bitpay/node-bufferput instead of the npm package?

Like we did with "browserify-bignum": "git://github.com/maraoz/browserify-bignum.git"

@jgarzik
Copy link
Contributor

jgarzik commented Feb 18, 2014

@maraoz That is an OK solution, too.

It is tiny and not widely used outside of bitcore, so reducing the number of dependencies by incorporating into bitcore seemed easy, efficient, and likely to permanently solve the reported problems.

@discretepackets
Copy link

@maraoz @jgarzik

Sorry, I'm not that familiar with participating in open source projects yet. (I was confused by who the "you" in "What do you think ..." was referring to.)

Should I make changes and submit a pull request?? Any changes made with the node-bufferput dependency should be applied to the insight repo too though.

@trapp
Copy link
Contributor

trapp commented Feb 24, 2014

I created PR #90 for issue #88. The solution is the same as @maraoz mentioned here so this PR might solve this issue as well.

@ryanxcharles
Copy link
Contributor Author

Pulled in PR #90 which fixed issue #88. Will regard this issue as closed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants