Hi Matt,
Thank you very much for taking the time to contribute your suggestions.
I've forwarded them to the development team and they also asked me to
express their thanks. A lot of the dead code, redundancies, and other items
that have been mentioned are there to keep things as simple as possible for
peer reviewed auditing and testing. As the codebase matures they've assured
me that the code will be cleaned up and proper work flow will be observed.
A lot of the comments are there as well because the code is updated several
times a day between the developers and the code on GitHub is currently
being pulled right from the devs in pre-alpha form.
Again, I would like to express our thanks for taking the time to review the
code and write this thoughtful post.
Regards,
Serena
On Monday, February 19, 2018 at 8:29:05 AM UTC-8, matthe...@gmail.com wrote:
>
> Hi Serena, here’s a code review.
>
> imports can be grouped with parenthesis:
>
> import (
> “fmt”
> “bytes”
> “testing”
> “encoding/hex”
>
> “github.com/deroproject/derosuite/config”
> )
>
> vars and consts are often grouped this way too. I see this is done
> sometimes but mostly not.
>
> package+type like block.Block could be better named. I’m wondering if
> these sub-packages would be better as files of package derosuite (or
> package dero, dero.Block makes more sense). Having a globals package says
> to me that these horizontal dependencies are unnecessary. The packages
> should be designed to present a godoc, not organize code for internal use.
>
> Perhaps consider struct embedding in cases like this if the exported
> methods aren't a problem:
>
> type Blockchain struct {
> …
> Mempool *mempool.Mempool
> …
> }
>
> type chain_data struct {
> hash crypto.Hash
> …
> }
>
> type Index_Data struct {
> …
> ECDHTuple ringct.ECdhTuple
> …
> }
>
> type CHAIN_CONFIG struct {
> …
> Network_ID uuid.UUID
> …
> Gensis_Block_Hash crypto.Hash
> …
> }
>
> type Connection struct {
> …
> Addr *net.TCPAddr
> …
> }
>
> Usually "func (chain *Blockchain) Block_Exists" would be "func (chain
> *Blockchain) BlockExists". The _ naming pattern makes the lines much longer
> than necessary.
>
> Relying on git to save the history is better than commenting out
> checked-in code, and relying on an issue tracker is better than having
> these dummy_test.go files.
>
> Instead of this in blockchain/blockheader.go:
>
> result.Height =
> result.Depth =
> result.Difficulty =
>
> Do this:
>
> result = BlockHeader_Print{
> Height: …,
> Depth: …,
> Difficulty: …,
> …
> }
>
> In blockchain/media_test.go the test should be split into cases and the
> test instead of repeating the test logic for every case.
>
> In cmd/explorer/explorer.go a local function "exitErr := func() {
> fmt.Fprintf… }" then:
>
> if err != nil {
> exitErr()
> return
> }
>
> may be better than using goto. The error message “Error occurred err %s”
> is redundant, just printing the error may be sensible enough. The audience
> for the error matters though so if a non-technical person may read it then
> perhaps the more verbose version is right.
>
> Editing cmd/explorer/templates.go may be easier if they are separate files
> to be parsed, although maybe having them as part of the binary is right.
>
> I’m wondering if the storage/interface.go Store type is necessary, usually
> an interface is smaller.
>
> In walletapi why not have "type keys" instead of "type _Keys"?
>
> Matt
>
> On Sunday, February 18, 2018 at 11:10:14 PM UTC-6, 867crypt...@gmail.com
> wrote:
>>
>> Hello, my name is Serena, I’m the Community Manager at a blockchain
>> project called Dero. We use a protocol called CryptoNote that was
>> originally written in C++. The developers at the Dero Project have
>> rewritten most of the (CryptoNote) codebase to golang and we would like to
>> start some discussion within the development and open source communities.
>>
>>
>> The core cryptography and functionally (rewritten in golang) is working
>> and available on GitHub for limited evaluation and testing. We would
>> genuinely value the feedback from this community. The following is what
>> I’ve posted to a couple other communities as well. (I would like to
>> emphasize that this project is currently pre-alpha but your input (the
>> google community) in particular is important to us.)
>>
>>
>> *DERO: Privacy + Smart Contracts*
>>
>>
>> Dero at present is a code fork of Monero (Helium Hydra) with the Bytecoin
>> CryptoNote protocol.
>>
>> Dero will be a completely new blockchain technology integrating the
>> CryptoNote protocol with new smart contract controls.
>>
>> *Dero is being rewritten from C++ to Golang (Google Language) to bring
>> together CryptoNote and smart contracts on the Dero blockchain.*
>>
>>
>> *CryptoNote protocol implementation in Golang is almost completed and
>> available on GitHub for basic testing/evaluation.*
>>
>>
>> *GitHub:* https://github.com/deroproject/derosuite (Golang version for
>> testing purposes only at this time)
>>
>>
>> *Bitcointalk:* https://bitcointalk.org/index.php?topic=2525508.0
>>
>>
>> Dero has a very welcoming community and we would enjoy the opportunity to
>> have you join us!
>>
>>
>> *FULL DISCLOSURE:Dero has a very welcoming community and we would enjoy
>> the opportunity to have you join us!*
>>
>>
>> *FULL DISCLOSURE:* My name is Serena, I’m the Community Manager at the
>> Dero Project
>>
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
