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.

Sign up for GitHub

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

Jump to bottom

Fix BufferWithTimeOrCount buffer emission #289

Closed
wants to merge 1 commit into from
Closed

Fix BufferWithTimeOrCount buffer emission #289

wants to merge 1 commit into from

Conversation

maguro
Copy link
Contributor

@maguro maguro commented Mar 22, 2021

The observable created with BufferWithTimeOrCount() should emit buffers
that are at most length count. The previous code erroneously used count
simply as a trigger for emission, resulting in the possibility of emitting
buffers larger than count.

@maguro maguro mentioned this pull request Mar 22, 2021
Open
@coveralls
Copy link

coveralls commented Mar 22, 2021 •
edited
Loading

Pull Request Test Coverage Report for Build 856

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%
Totals Coverage Status
Change from base Build 843: 0.0%
Covered Lines: 0
Relevant Lines: 0
💛 - Coveralls

@teivah teivah self-requested a review March 22, 2021 21:44
teivah
teivah requested changes Mar 22, 2021
View reviewed changes
Copy link
Member

@teivah teivah left a comment

Choose a reason for hiding this comment

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

It looks like Test_Observable_BufferWithTimeOrCountDoesNotExceedBufferSize is failing.
Btw, please rebase or merge your branch back from master. I've cleaned the CI.

@maguro
Copy link
Contributor Author

maguro commented Mar 29, 2021 •
edited
Loading

It looks like Test_Observable_BufferWithTimeOrCountDoesNotExceedBufferSize is failing.
Btw, please rebase or merge your branch back from master. I've cleaned the CI.

Done, btw.

@maguro maguro requested a review from teivah March 29, 2021 17:36
teivah
teivah reviewed Apr 6, 2021
View reviewed changes
observable_operator_test.go Show resolved Hide resolved
observable_operator.go Show resolved Hide resolved
@maguro maguro mentioned this pull request Apr 9, 2021
Merged
@maguro maguro requested a review from teivah April 13, 2021 17:34
@maguro
Fix BufferWithTimeOrCount buffer emission
3b3fc82 
The observable created with BufferWithTimeOrCount() should emit buffers
that are at most length count.  The previous code erroneously used count
simply as a trigger for emission, resulting in the possibility of emitting
buffers larger than count.
@dalianzhu
Copy link

This issue has not been resolved, is there a plan?

@maguro maguro closed this by deleting the head repository Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teivah teivah Awaiting requested review from teivah

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maguro @coveralls @dalianzhu @teivah