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

refactor: refactor getRange operation using scanner #154

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

foreverneverer
Copy link
Contributor

No description provided.

this.exception = exception;
}

public MultiGetResult convertMultiGetResult() {
Copy link
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
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
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
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.

for (int i = 0; i <= 4; i++) {
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
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