Skip to content

Conversation

@OsamaSBCrea
Copy link
Contributor

No description provided.

@OsamaSBCrea OsamaSBCrea requested a review from denisgursky May 4, 2023 18:30
src/rest.ts Outdated
timeout?: number;
};

export type PollingResult<T> =
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, why not just

export type PollingResult<T> = {
  done: boolean;
  result?: T;
};

src/rest.ts Outdated

export type PollOptions = {
overheadRate?: number;
startTime?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

startTime looks kinda weird, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So do we want to hardcode it in the pollWithOverhead function to Date.now, or we should just not expose it in the exec and pollTransaction options?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcode in pollWithOverhead

readonly = true,
tags: string[] = [],
interval = 1000, // 1 second
timeout = Number.POSITIVE_INFINITY,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be options?: PollOptions, to be consistent with the poll function

@OsamaSBCrea OsamaSBCrea requested a review from denisgursky May 9, 2023 13:48
@OsamaSBCrea OsamaSBCrea merged commit 3e290ef into main May 9, 2023
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