-
Notifications
You must be signed in to change notification settings - Fork 385
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
storage: Use compact.NodeID #2395
Conversation
6288fa7
to
e2e35b2
Compare
Codecov Report
@@ Coverage Diff @@
## master #2395 +/- ##
==========================================
+ Coverage 65.76% 65.77% +0.01%
==========================================
Files 107 107
Lines 7758 7749 -9
==========================================
- Hits 5102 5097 -5
+ Misses 2122 2120 -2
+ Partials 534 532 -2
Continue to review full report at Codecov.
|
return nodes, nil | ||
} | ||
|
||
// TODO(pavelkalinnikov): Allow nodes to be out of order. | ||
func nodesAreEqual(lhs []stree.Node, rhs []stree.Node) error { | ||
func nodesAreEqual(lhs, rhs []stree.Node) error { |
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.
Can we replace this with some variant of cmp.Diff? That did't exist when this was written + same for func below.
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.
The plan is to significantly refactor/simplify SubtreeCache
anyway, so I will likely revisit/delete this bit in follow-up PRs.
e2e35b2
to
0417598
Compare
0417598
to
29a7425
Compare
The old
NodeID
type is very complex. This change replaces it withthe simple
compact.NodeID
from the application layer all the way downto
SubtreeCache
, but the latter still uses the old type. In follow-upchanges it will be removed completely.
Part of #2378.
Checklist