-
Notifications
You must be signed in to change notification settings - Fork 42
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
change bitcore to dashcore and update dashcore-lib dependency #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments / couple questions, otherwise looks good!
lib/messages/commands/version.js
Outdated
@@ -37,7 +37,7 @@ function VersionMessage(arg, options) { | |||
this.nonce = arg.nonce || utils.getNonce(); | |||
this.services = arg.services || new BN(1, 10); | |||
this.timestamp = arg.timestamp || new Date(); | |||
this.subversion = arg.subversion || '/bitcore:' + packageInfo.version + '/'; | |||
this.subversion = arg.subversion || '/@dashcore/dashcore-p2p:' + packageInfo.version + '/'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not be "dashcore" per the documentation above? Although it would get confusing with "Dash Core" I guess...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! It's like a UserAgent info I guess. I'll put it to 'dashcore'. If it causes confusion we can still change it easily later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK, 👍
re-utACK @ e997ed6 |
Please use "Squash and Merge" option when merging this. Thanks! |
In the context preparing for a new Insight-api release, I found that dashcore-p2p is a dev dependency of dashcore-node. This is not very urgent, but would be nice to fix naming convention and dependency vulnerabilities (next PR) before release. I will bump the version once vulnerabilities are fixed.