-
Notifications
You must be signed in to change notification settings - Fork 29
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
chore: bunch of small refactors for improved devex #124
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just a slew of knits but nothing blocking 😄
server/server.go
Outdated
@@ -205,22 +203,23 @@ func (svr *Server) HandlePut(w http.ResponseWriter, r *http.Request) error { | |||
if len(key) > 0 && key != "put" { // commitment key already provided (keccak256) | |||
comm, err = commitments.StringToDecodedCommitment(key, ct) | |||
if err != nil { | |||
svr.log.Info("failed to decode commitment", "err", err, "key", key) | |||
w.WriteHeader(http.StatusBadRequest) | |||
err = fmt.Errorf("failed to decode commitment from key %v (commitment mode %v): %w", key, ct, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
knit - the commitment mode string could be isolated into a single for reference across error strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdym by "into a single"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merging this to unblock other work, but ping me to let me know what you meant and we can create another PR
server/server.go
Outdated
@@ -159,25 +155,26 @@ func (svr *Server) Health(w http.ResponseWriter, _ *http.Request) error { | |||
func (svr *Server) HandleGet(w http.ResponseWriter, r *http.Request) error { | |||
ct, err := ReadCommitmentMode(r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
knit and irrelevant to PR - ct implies CommitmentType
but it should probably shorthand notation cm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd/server/main.go
Outdated
var Version = "unknown" | ||
var Commit = "unknown" | ||
var Date = "unknown" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
knit - could encapsulate under a single var declaration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0d05a6d
to
2473f8e
Compare
624c815
to
405de62
Compare
See commit names for summary of changes.