Part of: Lint Go Code #49 #223
Conversation
Codecov Report
@@ Coverage Diff @@
## main #223 +/- ##
==========================================
+ Coverage 56.07% 56.73% +0.66%
==========================================
Files 74 73 -1
Lines 3089 2990 -99
==========================================
- Hits 1732 1696 -36
+ Misses 1186 1142 -44
+ Partials 171 152 -19
Continue to review full report at Codecov.
|
|
I like that you separated commits by package. We should not lint |
| @@ -6,21 +6,21 @@ import ( | |||
| "sync" | |||
| ) | |||
|
|
|||
| type key []byte | |||
There was a problem hiding this comment.
Intersection() is a public method which returns an array of items of Key type which itself is not public.
| @@ -60,15 +60,15 @@ func New() *Tree { | |||
| } | |||
| } | |||
|
|
|||
| func (dstTrie *Tree) Merge(srcTrieI merge.Merger) { | |||
There was a problem hiding this comment.
I know the linter says these names should be consistent (actually, I couldn't find the rule that enforces this), but I kinda feel in this particular case it's useful to have this src / dst distinction.
What do you all think about this? @AdrK @kolesnikovae
There was a problem hiding this comment.
I would go with consistent approach. Reader of this file should quickly learn what t stands for. Also, naming as dstTree and srcTree might suggests that both those vars came as an arguments and this method can work on two arbitrary trees, while it can only merge a tree provided by argument with itself and t denotes it ( like self in Python ).
There was a problem hiding this comment.
Yeah, good points, I think I'm in favor of t now
Datasource: Add query type switch
No description provided.