Skip to content

Add progress callback#58

Merged
PhenX merged 1 commit intomainfrom
feature/progress
Jun 8, 2025
Merged

Add progress callback#58
PhenX merged 1 commit intomainfrom
feature/progress

Conversation

@PhenX
Copy link
Copy Markdown
Owner

@PhenX PhenX commented Jun 8, 2025

Fixes #9

@PhenX PhenX marked this pull request as ready for review June 8, 2025 09:14
@PhenX PhenX merged commit 1429edc into main Jun 8, 2025
2 checks passed
return Math.Max(0, (int)CopyTimeout.TotalSeconds);
}

internal void HandleOnProgress(ref long rowsCopied)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I have really missed this PR. I think the ref is super weird here. Just to save a increment call on the provider level...

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

That's just a design choice on internal code, I find not so hard to understand and skips a few lines of code

/// <summary>
/// Number of rows after which the progress callback is invoked.
/// </summary>
public int? NotifyProgressAfter { get; set; }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is advantage of a nullable here?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No real advantages, I just could not decide which value was better

@SebastianStehle SebastianStehle deleted the feature/progress branch June 13, 2025 17:22
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.

Add a progress callback

2 participants