Skip to content

Conversation

Ryal02
Copy link

@Ryal02 Ryal02 commented Oct 7, 2025

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

@Ryal02 Ryal02 requested a review from a team as a code owner October 7, 2025 01:35
@Ryal02 Ryal02 requested review from dbkr and florianduros October 7, 2025 01:35
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Oct 7, 2025
Copy link
Contributor

@florianduros florianduros left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, few improvements over the documentation

Comment on lines +210 to +214
/**
* Optional. Timeout in milliseconds before the upload is aborted.
* Defaults to 30000 (30 seconds) if not specified.
*/
timeoutMs?: number;
Copy link
Contributor

@florianduros florianduros Oct 7, 2025

Choose a reason for hiding this comment

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

Suggested change
/**
* Optional. Timeout in milliseconds before the upload is aborted.
* Defaults to 30000 (30 seconds) if not specified.
*/
timeoutMs?: number;
/**
* Timeout in milliseconds before the upload is aborted.
* @default 30000 (30 seconds)
*/
timeoutMs?: number;


// set an initial timeout of 30s; we'll advance it each time we get a progress notification
let timeoutTimer = callbacks.setTimeout(timeoutFn, 30000);
// set an initial timeout (default to 30s); extendable via opts.timeoutMs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// set an initial timeout (default to 30s); extendable via opts.timeoutMs
// set an initial timeout (default to 30s); extendable via opts.timeoutMs
// we'll advance it each time we get a progress notification

@dbkr
Copy link
Member

dbkr commented Oct 7, 2025

You've checked the box to say you've signed off your changes and written tests, but I don't see either of these things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Enhancement Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants