Skip to content
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

Whisper Mail Server #3580

Merged
merged 22 commits into from Jan 31, 2017

Conversation

Projects
None yet
6 participants
@gluk256
Copy link
Contributor

gluk256 commented Jan 17, 2017

No description provided.

@mention-bot

This comment has been minimized.

Copy link

mention-bot commented Jan 17, 2017

@gluk256, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fjl and @karalabe to be potential reviewers.

@gluk256 gluk256 referenced this pull request Jan 17, 2017

Closed

Whisper Mail Server #3577

return e.DecodeBytes(raw)
}

func (e *Envelope) DecodeBytes(raw []byte) error {

This comment has been minimized.

Copy link
@fjl

fjl Jan 17, 2017

Contributor

Hmm, somehow this change has come back.

}
}

func extractIdFromEnode(s string) []byte {

This comment has been minimized.

Copy link
@fjl

fjl Jan 17, 2017

Contributor

You can use discover.ParseNode to simplify this function.

This comment has been minimized.

Copy link
@gluk256

gluk256 Jan 25, 2017

Author Contributor

fixed

"golang.org/x/crypto/pbkdf2"
)

const sizeOfInt = 4

This comment has been minimized.

Copy link
@fjl

fjl Jan 17, 2017

Contributor

This constant is a bit confusing because it's actually the size of a uint32. You can just use 4 everywhere instead of defining this. The size of uint32 won't change.

server *p2p.Server
shh *whisper.Whisper
done chan struct{}
input *bufio.Reader = bufio.NewReader(os.Stdin)

This comment has been minimized.

Copy link
@fjl

fjl Jan 17, 2017

Contributor

No need to make the type explicit here. It will be inferred.

@zelig
Copy link
Contributor

zelig left a comment

i wonder if there is any particular reason why you don't use the same cmd pattern that every other executable is using, namely:

  • gopkg.in/urfave/cli.v1 for config an launch
  • use go-ethereum/node / service
  • implement the actual functionality as a service (implementing NodeService interface)

You would then get proper help/usage for the commad
you could separate mailserver logic from the wrapper logic
you could (and should) write unit tests for server mail delivery
etc.


const sizeOfInt = 4

var (

This comment has been minimized.

Copy link
@zelig

zelig Jan 19, 2017

Contributor

should this not be const instead of var

This comment has been minimized.

Copy link
@gluk256

gluk256 Jan 19, 2017

Author Contributor

oh, thanx! this was a good one!

@zelig

zelig approved these changes Jan 27, 2017

Copy link
Contributor

zelig left a comment

somewhat reluctantly, given you didnt take my advice...
but lets move on

@fjl

fjl approved these changes Jan 31, 2017

@fjl fjl merged commit 690f6ea into ethereum:master Jan 31, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci Your tests passed on CircleCI!
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@fjl fjl removed the in progress label Jan 31, 2017

@karalabe karalabe added this to the 1.5.8 milestone Jan 31, 2017

farazdagi added a commit to status-im/go-ethereum that referenced this pull request Feb 23, 2017

@gluk256 gluk256 deleted the gluk256:23_mail branch Feb 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.