mvcc:add ut for Revisions/CountRevisions and remove RangeSince as it … #14124
Conversation
server/storage/mvcc/index.go
Outdated
| @@ -107,6 +105,9 @@ func (ti *treeIndex) unsafeVisit(key, end []byte, f func(ki *keyIndex) bool) { | |||
| }) | |||
| } | |||
|
|
|||
| // Revisions returns limited number of revisions from key(including) to end(excluding) | |||
| // at the given rev. The returned slice is sorted in the order of key. | |||
| // There is no limit if limit <= 0. | |||
There was a problem hiding this comment.
It seems also important that 'total' is not capped by the limit and reflects all the revisions.
There was a problem hiding this comment.
Comments:
includingtoincluded, andexcludingtoexcluded? This applies to the comment for methodCountRevisionsas well.- adding comment something like "The second return parameter
totalisn't capped by thelimit, and reflects all the expected revisions".
I'd defer to @spzala to correct the syntax of the comment.
There was a problem hiding this comment.
Thanks for review, update as you commented.
|
Overall looks good to me. Thank you. Leave @spzala to final decision. |
server/storage/mvcc/index.go
Outdated
| @@ -107,6 +105,9 @@ func (ti *treeIndex) unsafeVisit(key, end []byte, f func(ki *keyIndex) bool) { | |||
| }) | |||
| } | |||
|
|
|||
| // Revisions returns limited number of revisions from key(included) to end(excluded) | |||
| // at the given rev. The returned slice is sorted in the order of key. There is no limit if limit <= 0. | |||
| // The second return parameter total isn't capped by the limit, and reflects all the expected revisions. | |||
There was a problem hiding this comment.
Minor comment: replace total with total.
There was a problem hiding this comment.
Yes, minor but it would be good to make change considering how the parameter name. May be it will be more consistent with readability by changing the sentence to The second return parameter isn't capped by the limit and reflects the total number of revisions. Also, this way no need to markdown any variable?. cc @ahrtr
server/storage/mvcc/index.go
Outdated
| @@ -107,6 +105,9 @@ func (ti *treeIndex) unsafeVisit(key, end []byte, f func(ki *keyIndex) bool) { | |||
| }) | |||
| } | |||
|
|
|||
| // Revisions returns limited number of revisions from key(included) to end(excluded) | |||
| // at the given rev. The returned slice is sorted in the order of key. There is no limit if limit <= 0. | |||
| // The second return parameter total isn't capped by the limit, and reflects all the expected revisions. | |||
There was a problem hiding this comment.
Yes, minor but it would be good to make change considering how the parameter name. May be it will be more consistent with readability by changing the sentence to The second return parameter isn't capped by the limit and reflects the total number of revisions. Also, this way no need to markdown any variable?. cc @ahrtr
…is not used Signed-off-by: wayblink <wayasxxx@gmail.com>
It is kind of backport from etcd-io#14124. Signed-off-by: Wei Fu <fuweid89@gmail.com>
It is kind of backport from etcd-io#14124. Signed-off-by: Wei Fu <fuweid89@gmail.com>
…is not used
Signed-off-by: wayblink wayasxxx@gmail.com