Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Changed SaveAs[Async](string) to CopyTo[Async](Stream) #543

Closed
wants to merge 1 commit into from

Conversation

khellang
Copy link
Contributor

var inputStream = OpenReadStream();
inputStream.CopyTo(fileStream, DefaultBufferSize);
}
OpenReadStream().CopyTo(target, DefaultBufferSize);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we be disposing our stream? If we don't who does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I've seen it being disposed other places...

@khellang khellang force-pushed the save-to-stream branch 3 times, most recently from 2b0ca42 to 777eb18 Compare January 26, 2016 12:08
@khellang khellang changed the title Changed SaveAs(string) to Save(Stream) Changed SaveAs[Async](string) to CopyTo[Async](Stream) Jan 26, 2016
@muratg muratg added this to the 1.0.0-rc2 milestone Feb 4, 2016
@muratg
Copy link

muratg commented Feb 4, 2016

@Tratcher Could you have a look?

@Tratcher
Copy link
Member

Tratcher commented Feb 4, 2016

@lodejard @davidfowl What's the point of having IFormFile.CopyTo(Stream) when you already have IFormFile.OpenReadStream().CopyTo(Stream)?

Should we just remove SaveAs(string)?

@khellang
Copy link
Contributor Author

khellang commented Feb 4, 2016

What's the point of having IFormFile.CopyTo(Stream) when you already have IFormFile.OpenReadStream().CopyTo(Stream)?

Good point 😝👍

@Tratcher
Copy link
Member

Tratcher commented Feb 4, 2016

Hmm I guess it would actually be something like:

IFormFile.CopyTo(Stream output)
{
 using (var readStream = OpenReadStream())
 {
  reasStream.CopyTo(output);
 }
}

This might be worth it if it was actually important to dispose the readStream, but so far it isn't.

@Tratcher
Copy link
Member

Rebased and merged.

@Tratcher Tratcher closed this Feb 11, 2016
@khellang khellang deleted the save-to-stream branch February 12, 2016 00:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants