-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
Revert logic looks good to me.
Otherwise just some nits.
(children, children_height) | ||
}, | ||
None => { | ||
let children_height = stored_range.0; |
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.
nit: We could use a Range<usize>
type to make this a bit more natural rather than using a tuple?
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.
I'm not sure about the suggestion, but the tuple is (Vec, BlockNumber).
I can't use a Range here
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.
Then I'd use a struct
with named fields :)
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.
A few small nits, generally looks good 👍
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 👍 - would appreciate fewer .0
and .1
s but that's not blocking
Partially addresses #5396
This PR is a follow up of #5350 and addresses the revert of the
ApprovalVoting
subsystem.The operation api is similar to the one exposed by the
ChainSelection
subsystem.No particular blockers were found.