Skip to content

docs: rpclient is now part of main dcrd repo, fix code examples#970

Merged
davecgh merged 2 commits intodecred:masterfrom
markusrichter:docs
Jan 17, 2018
Merged

docs: rpclient is now part of main dcrd repo, fix code examples#970
davecgh merged 2 commits intodecred:masterfrom
markusrichter:docs

Conversation

@markusrichter
Copy link
Copy Markdown
Contributor

  • adjust links and example code to reflect that rpcclient is now part of the main dcrd repo
  • wire.NewShaHashFromStr has been moved to chainhash.NewHashFromStr
  • use mainnet port and genesis block hash

The prototype of the notification handlers OnBlockConnected and OnBlockDisconnected changed.
Any ideas how to process the blockHeader byte slice to make the output more meaningful?

Copy link
Copy Markdown
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It looks good overall, but I have a few recommendations inline.


```bash
Block count: 276978
2018/01/13 16:48:33 Block count: 203528
Copy link
Copy Markdown
Member

@davecgh davecgh Jan 14, 2018

Choose a reason for hiding this comment

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

I would prefer that the timestamp be left off. I'm aware that the log package adds one automatically, but it is going to constantly be out of date anyways, and it clutters up the example output in my opinion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, makes sense.

log.Printf("Block connected: %v (%d)", hash, height)
ntfnHandlers := rpcclient.NotificationHandlers{
OnBlockConnected: func(blockHeader []byte, transactions [][]byte) {
log.Printf("Block connected: %v %v", blockHeader, transactions)
Copy link
Copy Markdown
Member

@davecgh davecgh Jan 14, 2018

Choose a reason for hiding this comment

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

Would you mind changing these to %x and updating the sample output accordingly? This is leftover from when they used hashes instead of the actual data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, makes sense.

OnBlockDisconnected: func(hash *wire.ShaHash, height int32) {
log.Printf("Block disconnected: %v", hash, height)
OnBlockDisconnected: func(blockHeader []byte) {
log.Printf("Block disconnected: %v", blockHeader)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here regarding %x.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK

2014/05/12 20:33:19 Block connected: 000000000000000007dff1f95f7b3f5eac2892a4123069517caf34e2c417650d (300461)
2014/05/12 20:33:27 Client shutting down...
2014/05/12 20:31:27 Client shutdown complete.
2018/01/13 16:14:30 Client shutdown in 10 seconds...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency, if you decide to remove the timestamps from the others, please remove them from these as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK

Difficulty: 1.000000
Size (in bytes): 285
Num transactions: 1
2018/01/13 16:46:17 Hash: 298e5cc3d985bfe7f81dc135f360abe089edd4396b86d2de66b0cef42b21d980
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency, if you decide to remove the timestamps from the others, please remove them from these as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK

Adjust links and  example code to reflect that.
Also update example output.
- wire.NewShaHashFromStr has been moved to chainhash.NewHashFromStr
- use mainnet port and genesis block hash
@markusrichter
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, I addressed your remarks.

@davecgh davecgh merged commit 239d4c8 into decred:master Jan 17, 2018
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.

2 participants