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

Add a leaveOpen parameter to the ZipFile constructor, used to set the… #302

Merged
merged 2 commits into from
Jan 21, 2019

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Jan 13, 2019

… default value of isStreamOwner

Related to #144 - This change adds an extra version of the constructor for ZipFile which takes a 'leaveOpen' flag, which can be set to set the isStreamOwner flag during construction rather than after, which can help avoid unwanted stream disposal if the constructor throws (the param is called leaveOpen rather than isStreamOwner to match the naming of various other .net libs, and could be changed).

[WIP] because it could do with some tests if deemed a reasonable thing to do.

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@piksel
Copy link
Member

piksel commented Jan 14, 2019

Good choices! Some simple unit tests for new constructors should probably be added.

@Numpsy Numpsy changed the title [WIP] Add a leaveOpen parameter to the ZipFile constructor, used to set the… Add a leaveOpen parameter to the ZipFile constructor, used to set the… Jan 14, 2019
@Numpsy
Copy link
Contributor Author

Numpsy commented Jan 14, 2019

added a few tests

@piksel piksel merged commit 3b4966c into icsharpcode:master Jan 21, 2019
@piksel
Copy link
Member

piksel commented Jan 21, 2019

Great quality PR! Thank you.

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