Skip to content

Conversation

@carlosmiei
Copy link
Collaborator

@carlosmiei carlosmiei requested a review from Copilot May 26, 2025 13:51
@carlosmiei carlosmiei merged commit 40c65da into master May 26, 2025
1 check failed
Copy link

Copilot AI left a 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 updates the return types for the futuresCancelMultipleOrders and futuresCancelAll functions to better reflect the Binance API responses.

  • Added a new Response interface in src/types.ts.
  • Updated the return type for futuresCancelMultipleOrders to a union of FuturesOrder and Response.
  • Changed the return type for futuresCancelAll to Response.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/types.ts Added a generic Response interface for API responses.
src/node-binance-api.ts Updated return types for futures cancel functions.
Comments suppressed due to low confidence (3)

src/types.ts:47

  • [nitpick] The name 'Response' is very generic and may conflict with built-in types or other usage. Consider renaming it to a more descriptive name like 'ApiResponse' to enhance clarity and avoid ambiguity.
export interface Response {

src/node-binance-api.ts:4346

  • Ensure that the union type (FuturesOrder|Response) accurately reflects all possible outcomes of this API call. If error responses follow the Response structure, consider updating the associated documentation to clarify these conditions.
async futuresCancelMultipleOrders(symbol: string, params: Dict = {}): Promise<(FuturesOrder|Response)[]> {

src/node-binance-api.ts:4382

  • Double-check that the Response type sufficiently captures the expected outcomes for canceling all orders. If the underlying API returns detailed order cancellation information, a more specific type might be warranted.
async futuresCancelAll(symbol: string, params: Dict = {}): Promise<Response> {

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.

2 participants