Skip to content

refactor: add client and remove basic#1

Merged
BlackHole1 merged 1 commit intomainfrom
improve-code
Oct 31, 2025
Merged

refactor: add client and remove basic#1
BlackHole1 merged 1 commit intomainfrom
improve-code

Conversation

@BlackHole1
Copy link
Member

No description provided.

Signed-off-by: Kevin Cui <bh@bugs.cc>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Summary by CodeRabbit

  • New Features

    • Introduced Client class for SDK initialization
    • Added instanceName field to configuration
  • Breaking Changes

    • Operator classes now accept Client instance instead of configuration objects
    • Simplified configuration interface; removed factory-based patterns
  • Chores

    • Updated package metadata with description and keywords

Walkthrough

This pull request refactors the SDK's architectural pattern from inheritance-based to composition-based dependency injection. It removes the Basic base class, introduces a new Client wrapper class that encapsulates a Request instance, and updates all operator classes (BatchGetRow, BatchWriteRow, DeleteRow, GetRange, GetRow, PutRow, UpdateRow) to accept a Client instance via constructor instead of inheriting from Basic. Type definitions are consolidated, with RequestConfig renamed to ClientConfig and unused abstractions (RequestFactory, OperatorConfig) removed. The exports in index.ts and package.json metadata are updated accordingly.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Operator class refactoring consistency: Verify that all seven operator classes (batch-get-row, batch-write-row, delete-row, get-range, get-row, put-row, update-row) correctly receive the Client instance and access this.client.request.do(...) without functional regressions
  • Type rename propagation: Confirm RequestConfigClientConfig rename is applied consistently across src/request.ts, src/type.ts, and all imports throughout the codebase
  • Public API boundary: Verify the new Client class properly exposes the request interface and that all constructor parameter types match the updated exports
  • Import/export accuracy: Validate src/index.ts exports are complete and correct, including the removal of Basic and RequestConfig/RequestFactory and the addition of Client and OperationType

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive No pull request description was provided by the author. While the check is lenient and only requires the description to be related to the changeset, the absence of any description content makes it impossible to evaluate whether it is on-topic or off-topic. This creates ambiguity about whether the check can be properly assessed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title follows the required format <type>(<scope>): <subject> with "refactor" as the type and "add client and remove basic" as the subject. The title is directly related to the main changes in the changeset: the introduction of a new Client class and removal of the Basic class, along with the refactoring of all dependent operator classes to use composition with Client instead of inheritance from Basic. The title accurately summarizes the core purpose of this refactor.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/client.ts (1)

5-8: Prefer readonly for the request handle.

Marking request as readonly prevents accidental reassignment and better communicates the intent that callers should treat it as immutable.

-    public request: Request;
+    public readonly request: Request;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c62513 and 3d50948.

📒 Files selected for processing (13)
  • package.json (1 hunks)
  • src/client.ts (1 hunks)
  • src/index.ts (3 hunks)
  • src/operator/basic.ts (0 hunks)
  • src/operator/batch-get-row.ts (3 hunks)
  • src/operator/batch-write-row.ts (3 hunks)
  • src/operator/delete-row.ts (3 hunks)
  • src/operator/get-range.ts (3 hunks)
  • src/operator/get-row.ts (3 hunks)
  • src/operator/put-row.ts (3 hunks)
  • src/operator/update-row.ts (3 hunks)
  • src/request.ts (2 hunks)
  • src/type.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/operator/basic.ts
🧰 Additional context used
🧬 Code graph analysis (10)
src/request.ts (1)
src/type.ts (1)
  • ClientConfig (3-8)
src/operator/update-row.ts (1)
src/client.ts (1)
  • Client (4-9)
src/operator/put-row.ts (2)
src/index.ts (2)
  • PutRow (62-62)
  • Client (2-2)
src/client.ts (1)
  • Client (4-9)
src/operator/delete-row.ts (1)
src/client.ts (1)
  • Client (4-9)
src/operator/batch-write-row.ts (2)
src/index.ts (2)
  • BatchWriteRow (21-21)
  • Client (2-2)
src/client.ts (1)
  • Client (4-9)
src/type.ts (1)
src/index.ts (1)
  • ClientConfig (115-115)
src/operator/get-row.ts (2)
src/index.ts (2)
  • GetRow (52-52)
  • Client (2-2)
src/client.ts (1)
  • Client (4-9)
src/operator/batch-get-row.ts (1)
src/client.ts (1)
  • Client (4-9)
src/client.ts (2)
src/request.ts (1)
  • Request (11-53)
src/type.ts (1)
  • ClientConfig (3-8)
src/operator/get-range.ts (2)
src/index.ts (2)
  • GetRange (44-44)
  • Client (2-2)
src/client.ts (1)
  • Client (4-9)
🔇 Additional comments (8)
src/operator/delete-row.ts (2)

28-30: Clean composition pattern adopted.

The constructor now accepts a Client instance instead of extending Basic, which is a good architectural improvement favoring composition over inheritance. Note that this is a breaking change—existing users will need to instantiate a Client and pass it to DeleteRow.


65-71: Request delegation correctly implemented.

The call to this.client.request.do() correctly delegates to the injected Client instance, maintaining the same behavior as before while using the new composition pattern.

src/operator/get-range.ts (1)

34-36: Refactoring applied consistently.

The composition pattern is correctly implemented in GetRange, matching the changes in other operator files. The constructor accepts a Client instance and the do method properly delegates to this.client.request.do().

Also applies to: 87-93

src/operator/update-row.ts (1)

35-37: Composition pattern correctly applied.

UpdateRow follows the same clean refactoring as the other operators. The constructor accepts a Client and the do method correctly delegates through this.client.request.do().

Also applies to: 68-74

src/operator/get-row.ts (1)

33-35: Refactoring applied consistently.

The implementation in GetRow matches the pattern used across all operator files. The dependency injection of Client and the subsequent delegation to this.client.request.do() are correct.

Also applies to: 81-87

src/operator/batch-get-row.ts (1)

47-49: Consistent refactoring in batch operation.

BatchGetRow correctly implements the composition pattern with Client injection. The delegation to this.client.request.do() maintains the expected behavior.

Also applies to: 96-102

src/operator/batch-write-row.ts (1)

64-66: Final operator successfully refactored.

BatchWriteRow completes the consistent refactoring across all operator classes. The constructor correctly accepts a Client instance and the do method properly delegates to this.client.request.do().

Also applies to: 110-116

src/type.ts (1)

3-8: Verification confirms instanceName is properly utilized—no changes needed.

The instanceName field is actively used in the Request class (line 21 of src/request.ts) where it's passed as the H_OTS_INSTANCE_NAME header. The field is not silently ignored and is properly integrated alongside other configuration parameters.

@BlackHole1 BlackHole1 merged commit a0a638a into main Oct 31, 2025
2 checks passed
@BlackHole1 BlackHole1 deleted the improve-code branch October 31, 2025 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant