Refactor to partially remove deprecated Value classes usage
#2328
Conversation
Torch3333
changed the title
Torch3333
changed the title
Torch3333
changed the title
Value classes usage
Torch3333
force-pushed
the
refactor_to_remove_values_partially
branch
from
35f0169 to
4281076
Compare
Torch3333
force-pushed
the
refactor_to_remove_values_partially
branch
from
4281076 to
68f46ef
Compare
| col7Value); | ||
| } | ||
|
|
||
| private void assertResult( |
There was a problem hiding this comment.
I refactored this class's integration test cases assertion in a common method.
| // Arrange | ||
| Scan scan = prepareScan(partitionKeyType, partitionKeyValue); | ||
|
|
||
| // Act Assert | ||
| try (Scanner scanner = storage.scan(scan)) { | ||
| Optional<Result> result = scanner.one(); | ||
| Optional<Result> optResult = scanner.one(); |
There was a problem hiding this comment.
[trivial] You used resultOpt at https://github.com/scalar-labs/scalardb/pull/2328/files#diff-d6e999e6390b76c30190d9b158a27634af73f3db28349b60430a5378bf681283R271. Using unified naming convention would be better? (Either of resultOpt or optResult is fine with me)
There was a problem hiding this comment.
Thank you! I will use optResult.
At first, I used resultOpt but changed midway to optResult and forgot to rename that one.
There was a problem hiding this comment.
Revised in 4031a79
|
@komamitsu @brfrn169 @feeblefakie I will move the PR back to draft and notify you again once it is ready for review. |
This reverts commit 93ce2fa
|
@komamitsu @brfrn169 @feeblefakie I reverted some changes with 97f5c90 that should not be merged into the master branch at the moment. |
Description
ScalarDB core still uses the deprecated Value interface heavily. Since I don't want to have to implement the time-related types for this deprecated interface such as
TimestampValue,TimeValue, etc. I needed to replace the usage of theValueinterface with the Column interface.I refactored partially the integration test and some parts of the core main package.
I will refactor unit tests for the same reason in a later PR if necessary.
Related issues and/or PRs
Changes made
Refactor some integration tests and some classes of the core/main package to replace the usage of the Value interface by the Column interface.
Checklist
Additional notes (optional)
N/A
Release notes
N/A