Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return number of processed items and Abort error #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alinz
Copy link

@alinz alinz commented Feb 13, 2020

@matryer Thanks for this little code which makes go shine.

I have decided to send a PR rather than creating an issue. I just wanted to know your opinions.

I have separate them in two commits.

The first one emphasized on returning Abort error. I feel like returning an Abort is the same as all items has been processed. There is no way to indicate whether all is done or it is been aborted.

The second commit focus on returning the number of processed items in batch. Similar to io.Reader.Read() which returns the number of bytes that has been read.

any thoughts on that?
cheers

@alinz alinz requested a review from matryer February 14, 2020 15:56
@matryer
Copy link
Collaborator

matryer commented Feb 14, 2020

@alinz I think you're absolutely right that you would want the Abort error returned. @dahernan and I discussed returning the count and I think it's a nice addition. I'll look over this tomorrow in detail. Thanks.

@alinz alinz requested review from matryer and removed request for matryer February 20, 2020 20:52
@alinz
Copy link
Author

alinz commented Feb 29, 2020

@matryer just wanted to see if this PR worth merging? if not let's close it.

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