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

Added pubkey support to e4client #24

Merged
merged 5 commits into from Feb 3, 2020
Merged

Added pubkey support to e4client #24

merged 5 commits into from Feb 3, 2020

Conversation

daeMOn63
Copy link
Member

No description provided.

Copy link
Contributor

@odeke-em odeke-em left a comment

Choose a reason for hiding this comment

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

Thank you for this change @daeMOn63!

A couple of comments:
a) Let's refactor out the repetitive fmt.Print.. and os.Exit(1) into a helper utility logAndExit that looks like this

func logAndExit(fmtStr string, args ...interface{}) {
     fmt.Printf(fmtStr, ...args)
     os.Exit(1)
}

b) Replace os.Open, defer (*os.File).Close, ioutil.ReadAll with ioutil.ReadFile

cmd/e4client/e4client.go Outdated Show resolved Hide resolved
cmd/e4client/e4client.go Outdated Show resolved Hide resolved
cmd/e4client/e4client.go Outdated Show resolved Hide resolved
@daeMOn63
Copy link
Member Author

@odeke-em instead of logAndExit I went the log.Fatal way you suggested before on the keygen, thanks for pointing this out, and the other tips !

@daeMOn63 daeMOn63 changed the title Added pubkey support to e4client WIP - Added pubkey support to e4client Dec 30, 2019
@daeMOn63
Copy link
Member Author

WIP added - we can hold on pubkey support for after the first release

Copy link
Contributor

@odeke-em odeke-em left a comment

Choose a reason for hiding this comment

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

Just one thing to look at them LGTM.

l.Printf("failed to generate key: %v", err)
return
}
l.Printf("public key: %x", key.Public())
Copy link
Contributor

Choose a reason for hiding this comment

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

This code will unconditionally print "public key" and is going to crash if err != nil and key == nil.
You need to add a return on line 183.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks!

Copy link
Contributor

@odeke-em odeke-em left a comment

Choose a reason for hiding this comment

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

LGTM and thank you @daeMOn63!

@daeMOn63 daeMOn63 changed the title WIP - Added pubkey support to e4client Added pubkey support to e4client Feb 3, 2020
@daeMOn63 daeMOn63 merged commit 09fad8f into develop Feb 3, 2020
@diagprov diagprov deleted the fb/e4client-pubkey branch February 3, 2020 16:30
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.

None yet

3 participants