DHT PR #25
DHT PR #25
Conversation
|
Okay, much more manageable than last one-- merging what's there is useful. |
| u.DOut("identify: Got node id: %s", remote.ID.Pretty()) | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
maybe the identify pkg could also include a PublicKey func that requests the remote peer's pub key. Handshake could do it if the key's not known locally yet. (RTT lost, prob worth sending along with the id).
| // kBuckets define all the fingers to other nodes. | ||
| Buckets []Bucket | ||
| Buckets []*Bucket | ||
| bucketsize int | ||
| } |
There was a problem hiding this comment.
maybe worth moving the RoutingTable and Bucket structs into a subpackage. (that way their functionality can be tested independently, and the they can be reused more modularly)
|
Okay! first set of comments added, from basic reading of the code. Still need to sink in and study/test how it works. Modularizing more will help here -- moving the RoutingTable + Bucket into a subpackage and test that thoroughly on its own. on the code(not the dht logic) One big general note on programming on panics. You're using panics everywhere as one would normally use Also:
|
|
Yeaahhh... I use panics heavily while im writing code to remind myself of what still needs to be done, ala "I dont want to worry about writing this logic yet, so ill panic instead of writing it out for now". My personal philosophy on real panic use is only in cases of programmer error, a user should not be able to trigger panics. |
|
Also, thank you for pointing out the formatting problem, it led me to finding out that my vim wasnt set up properly on my new machine. |
|
Merging this in, we'll worry about evolving it from master. |
| // Read in all messages from swarm and handle them appropriately | ||
| // NOTE: this function is just a quick sketch | ||
| func (dht *IpfsDHT) handleMessages() { | ||
| u.DOut("Begin message handling routine") | ||
|
|
||
| checkTimeouts := time.NewTicker(time.Minute * 5) |
There was a problem hiding this comment.
Numbers like these should be consts
gx publish 1.1.27
This PR will be for CR and conversation on the DHT.