Various lints #80
Various lints #80
Conversation
* VerifyItem takes no index; Return keys/values in range; Fix * compute hash from rangeproof * Require Verify() before Verify*() * Review fixes from cosmos#75 * Bump version, update changelog
-ineffassign, misspell, (some) golint
- _PathToLeaf -> _pathToLeaf - Package iavl description - remove obsolete proof-types from doc
- comment PrintTree - StringIndented -> stringIndented
- if block ends with a return statement, so drop this else and outdent its block (golint)
- if block ends with a return statement, so drop this else and outdent its block (golint) - remove unused named returns - more idiomatic go code ( += 1 -> ++ etc)
- if block ends with a return statement, so drop this else and outdent its block (golint) - remove unused named returns - updated/improved comment
- consistent receiver names - omit type from declaration
proof.go
Outdated
| @@ -144,10 +144,14 @@ func (pln proofLeafNode) Hash() []byte { | |||
| // a path to the least item. | |||
| func (node *Node) PathToLeaf(t *Tree, key []byte) (PathToLeaf, *Node, error) { | |||
| path := new(PathToLeaf) | |||
| val, err := node._PathToLeaf(t, key, path) | |||
| val, err := node._pathToLeaf(t, key, path) | |||
There was a problem hiding this comment.
why underscore in the beginning?
There was a problem hiding this comment.
Was there before. I think Jae uses them to mark recursive helpers. I can remove it here but there are many other places.
| // Right Left Case | ||
| var rightOrphaned *Node | ||
| } | ||
| // Right Left Case |
There was a problem hiding this comment.
I'd advocate that sometimes if then else does make perfect sense and this is one of those cases
if left {
} else {
(right)
}
NOT
if left {
}
(right)
There was a problem hiding this comment.
I kinda agree. It makes more sense here then in many other places. I think the point is that there is a return in the first case (which makes the else obsolete semantically). Can reintroduce if you think that is important.
I would rather suggest to refactor this such that the return statement(s) is(/are) outside of the if-then-else. Something I really like about (idiomatic) go is that code usually looks very similar. For me an if left { // ...} else { //right } would look like there are no return statements to expect there. What do you think?
| } | ||
| proof.rootVerified = true |
There was a problem hiding this comment.
note: this is the good case where else is not needed indeed
proof_range.go
Outdated
| @@ -179,7 +177,7 @@ func (proof *RangeProof) Verify(root []byte) error { | |||
| return err | |||
| } | |||
|
|
|||
| func (proof *RangeProof) verify(root []byte) (err error) { | |||
| func (proof *RangeProof) verify(root []byte) (error) { | |||
There was a problem hiding this comment.
brackets around error can be removed
- remove brackets from single return val - remove underscore from method name Signed-off-by: Liamsi <[email protected]>
* 'develop' of github.com:danil-lashin/iavl: Prep Release 0.11.0 (cosmos#111) Remove architecture dependent getter functions (cosmos#96) Use 8 bytes to store int64 components of database keys (cosmos#107) Update to CircleCI 2.0 (cosmos#108) Release 0.10.0: Update Changelog and bump version (cosmos#99) delete empty file (relict from merging master into develop) Mutable/Immutable refactor and GetImmutable snapshots (cosmos#92) Remove unreachable code Remove unused variable dep: Change tendermint dep to be ^v0.22.0 (cosmos#91) Fix benchmark scripts for current code (cosmos#89) release v0.9.2 (cosmos#82) Various lints (cosmos#80) Jae/rangeprooffix (cosmos#75)
* adhere to: ineffassign, misspell, golint * unexport helper function & godoc compatibility for package comments * _PathToLeaf -> pathToLeaf * remove obsolete proof-types from doc * comment exported method and unexport debug helper method * remove unused private methods * remove unnecessary else blocks (golint): * remove unused named returns * minor changes to make code more idiomatic * consistent receiver names * omit type from declaration * remove brackets from single return val Signed-off-by: Liamsi <[email protected]>
As it is planned to merge iavl into cosmos-sdk, we need to conform to the lints used there:
https://github.com/cosmos/cosmos-sdk/blob/3f438cdbc2ed6b462e3e146403f836c8bb680317/Makefile#L104-L107
This PR contains most changes necessary for this.
It does not contain changes which would silence warnings a la:
comment on exported method Node.PathToLeaf should be of the form "PathToLeaf ..." (golint)I think in tendermint we do not lint for these, too.
It also merges master into develop (maybe should be a separate change though).
Supersedes #58