-
Notifications
You must be signed in to change notification settings - Fork 39
Remove deprecated API usages (Part 2) #2957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
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.
Pull Request Overview
This PR removes deprecated API usage by migrating from deprecated constructor-based patterns to builder patterns for ScalarDB operations. This includes removing deprecated methods like withValue()
, withProjection()
, forNamespace()
etc., and updating to use Put.newBuilder()
, Get.newBuilder()
, Scan.newBuilder()
, and Delete.newBuilder()
.
Key changes include:
- Replacing deprecated operation constructors with builder patterns
- Removing deprecated method calls like
withValue()
,forNamespace()
,forTable()
- Updating from
ScanAll
toScan
with.all()
builder method
Reviewed Changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
TwoPhaseConsensusCommitSpecificIntegrationTestBase.java | Updated test methods to use builder patterns for Put/Get/Scan/Delete operations |
ConsensusCommitSpecificIntegrationTestBase.java | Migrated operation constructors to builder patterns in integration tests |
ConsensusCommitNullMetadataIntegrationTestBase.java | Updated ScanAll usage to Scan with builder pattern |
TwoPhaseCommitTransactionIntegrationTestBase.java | Replaced deprecated constructors with builder patterns in transaction tests |
DistributedTransactionIntegrationTestBase.java | Updated operation creation to use builder patterns instead of deprecated constructors |
DistributedTransactionCrossPartitionScanIntegrationTestBase.java | Migrated projection method to use builder pattern |
DistributedTransactionAdminIntegrationTestBase.java | Updated Put operation to use builder pattern with proper value setters |
DistributedStorageIntegrationTestBase.java | Migrated storage operations to builder patterns |
DistributedStorageConditionalMutationIntegrationTestBase.java | Updated conditional mutation operations to use builder patterns |
DistributedStorageAdminIntegrationTestBase.java | Migrated admin operations to builder patterns |
JdbcTransactionManagerTest.java | Updated test operations to use builder patterns |
TwoPhaseConsensusCommitTest.java | Migrated test helper methods to builder patterns |
TransactionTableMetadataManagerTest.java | Updated Get operations to use builder patterns |
SnapshotTest.java | Migrated snapshot test operations to builder patterns |
SnapshotKeyTest.java | Updated test operations to use builder patterns |
RollbackMutationComposerTest.java | Migrated composer test operations to builder patterns |
PrepareMutationComposerTest.java | Updated mutation composer operations to builder patterns |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Summary of Changes
Hello @brfrn169, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on modernizing the API by replacing deprecated operation constructors with a builder pattern. This change affects how Put
, Get
, Scan
, and Delete
operations are instantiated throughout the codebase, leading to more consistent and readable code. The refactoring extends to various storage and transaction modules, ensuring that the system leverages the benefits of immutable operation objects. The comprehensive updates across both core logic and test suites underscore a commitment to maintaining high code quality and API usability.
Highlights
- API Modernization: The primary change in this pull request is the migration from direct constructor usage for
Put
,Get
,Scan
, andDelete
operations to a more modern and flexible builder pattern. This enhances API consistency and code readability. - Deprecation Removal: Deprecated constructors for
Mutation
operations have been removed, ensuring that new code adheres to the updated builder-based API. - Code Refactoring: Utility methods and core logic across various storage and transaction modules have been refactored to seamlessly integrate with the new builder pattern, promoting immutability and cleaner code.
- Test Suite Updates: All relevant integration and unit tests have been updated to reflect the API changes, ensuring continued correctness and stability of the codebase.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request is a large-scale refactoring to remove deprecated API usages, primarily replacing old-style operation constructors with the builder pattern. The changes are extensive and touch many files, including core logic and tests. The new code is more readable and promotes immutability, which is a significant improvement in code quality. The changes appear to be correct and consistent with the PR's goal. I've identified a couple of areas for potential improvement regarding code duplication and conciseness.
core/src/test/java/com/scalar/db/storage/cassandra/BatchHandlerTest.java
Outdated
Show resolved
Hide resolved
/gemini review |
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.
Code Review
This pull request is a large but valuable refactoring effort to remove deprecated API usages, specifically replacing operation constructors with the builder pattern. The changes are widespread across the codebase, including core logic and numerous test files, and appear to be correctly implemented. This move towards builders is a positive step for maintainability and API clarity. I particularly appreciate the improvements towards immutability in classes like CrudHandler
and ScalarDbUtils
, where methods that previously mutated their arguments now create new objects. The test updates are also consistent with these changes. Overall, this is a solid and well-executed refactoring.
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, thank you!
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! 👍
Description
This PR removes deprecated API usages. The changes were primarily made using Claude Code.
Related issues and/or PRs
Changes made
Checklist
Additional notes (optional)
N/A
Release notes
N/A