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

2335-MemoryFileWriteStream #4601

Open
wants to merge 1 commit into
base: Pharo8.0
from

Conversation

@hilaire
Copy link
Contributor

hilaire commented Sep 15, 2019

Fixes for #2335-MemoryFileWriteStream-did-not-understand-nextputAllstartingAt
Hopefully without unwished commits.

@stale

This comment has been minimized.

Copy link

stale bot commented Oct 5, 2019

It is hard to review old PRs so this PR has been marked as stale since there has been no activity the last 20 days. It will be closed in 10 days if no further activity occurs. A simple comment will reactivate the PR, but please also consider updating the PR to the latest SNAPSHOT build to make it easier for reviewers.

@stale stale bot added the stale label Oct 5, 2019
@hilaire

This comment has been minimized.

Copy link
Contributor Author

hilaire commented Oct 6, 2019

Waiting for review

@stale stale bot removed the stale label Oct 6, 2019
@Ducasse

This comment has been minimized.

Copy link
Member

Ducasse commented Oct 8, 2019

I do not understand why next is cancelled. Even in memory we can move one by one, no?

@hilaire

This comment has been minimized.

Copy link
Contributor Author

hilaire commented Oct 9, 2019

Well, I did not look at checking the behavior of next because at the first place MemoryFileWriteStream was a subclass of an Object and next was not implemented. Making it a subclass of Stream fix the error, but I can't say then if the inherited next is meaningful or working. So I prefere to stick it as shouldnotiomplement.
What should we do?

@svenvc

This comment has been minimized.

Copy link
Contributor

svenvc commented Oct 9, 2019

I also do not understand: this looks totally wrong to me.

I've said this before, a simple WriteStream on a ByteArray is a perfect 'memory file write stream'. Why do we even need something else ? I would like to understand that first.

@hilaire

This comment has been minimized.

Copy link
Contributor Author

hilaire commented Oct 9, 2019

So is the idea more like MemoryFileWriteStream is not needed and should be deprecated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.