Skip to content
This repository was archived by the owner on May 10, 2022. It is now read-only.

refactor: refactor getRange operation using scanner#154

Draft
foreverneverer wants to merge 20 commits intoXiaoMi:masterfrom
foreverneverer:refactor-range-one
Draft

refactor: refactor getRange operation using scanner#154
foreverneverer wants to merge 20 commits intoXiaoMi:masterfrom
foreverneverer:refactor-range-one

Conversation

@foreverneverer
Copy link
Copy Markdown
Contributor

No description provided.

this.exception = exception;
}

public MultiGetResult convertMultiGetResult() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment for these two functions

}
sortKeysResult.allFetched = result.allFetched;
return sortKeysResult;
ScanOptions scanOptions = new ScanOptions();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we reserver the old interface and add a new interface for these code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, if like multiGet, origin multiGetSortKeys should also be retained and only marked Deprecated. but the API has been refactored by scan in previous PR and some user has use it, here I just keep it. @neverchanje @hycdong can give some suggestion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is okay to update code in function multiGetSortKeys, because it is already implemented by scan.

Assertions.assertEquals("persistent_" + i, new String(caseD1.keys.get(i)));
Assertions.assertEquals("persistent_" + i, new String(result1.keys.get(i)));
}
// case D1: maxFetchCount < 0, return all valid record
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case A,B,C have been moved to TestRange.java, how about update comment for case D1?

@foreverneverer foreverneverer marked this pull request as draft April 6, 2021 02:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants