Issue #1 Add Headers to all go files, and update the LICENSE #2
Conversation
|
|
||
| Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: |
There was a problem hiding this comment.
I'd suggest use of MIT or other acknowledged, reviewed, and community accepted license rather than rolling your own. Of course, integration with factomd would be slightly easier with MIT
| "math/rand" | ||
| "time" | ||
|
|
||
| log "github.com/sirupsen/logrus" |
There was a problem hiding this comment.
Logrus is pretty heavy as a logging package. I see that the live feed uses it too, but I am not entirely sure its value justifies its overhead.
Here is the results from one set of benchmarks
There was a problem hiding this comment.
I picked logrus because that's one of the logging packages used throughout factomd and the goal was to keep is as consistent as I could to aid integration and transfer experience of people familiar with factomd.
The benchmarks are pretty bad, though. Are there any plans to replace factomd's logrus with something else? Would switching out the logging package still be compatible with factomd's existing infrastructure / elk stash?
| ) | ||
|
|
||
| // Configuration defines the behavior of the gossip network protocol | ||
| type Configuration struct { |
There was a problem hiding this comment.
Looks like we are hardwired to TCP. Can we support UDP? Would be great if we could support simulation nodes. The current code puts simulation above quite a bit of logic in the gossip network that we could test if we made the protocol switchable.
There was a problem hiding this comment.
I'm not sure why you need UDP for simulation nodes. Everything in the factomd network is currently TCP based, so the only way for this to be a compatible drop-in replacement is also TCP. I've been simulating networks by running each simulated node on a different loopback ip (127.xxx.xxx.xxx), giving me over 16million potential nodes.
Looking into UDP support is something I've had on the long-term roadmap but it's not something I planned for the initial release. It would be interesting to see how well running a dual network (TCP and UDP) would work though.
I think it's an outdated style coming from the old days of source distribution. GitHub states explicitly in their terms of service that all files inside a repo, unless noted otherwise, are subject to the license in the LICENSE file. This is definitely a preference though and I'd be interested in your and other people's input on this. |
|
Good catch on the CRLF issue |
Generally having a header on each file is done, but also provides a means of commenting on other issues.