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

Save temporary files to a temporary directory #325

Merged
merged 1 commit into from Nov 8, 2017

Conversation

@aeroastro
Copy link
Contributor

@aeroastro aeroastro commented Apr 14, 2017

I would like to save temporary files to a temporary directory.

Because the primary objective of @temp_file in Zip::StreamableStream is to temporarily hold data of an entry as a file, the upper directory does not need to be the current directory.

This patch set the directory to default Dir.tmpdir. As a result, we can stop temporary files from filling up current directory.

c.f. http://ruby-doc.org/stdlib-2.4.1/libdoc/tempfile/rdoc/Tempfile.html

@coveralls
Copy link

@coveralls coveralls commented Apr 14, 2017

Coverage Status

Coverage decreased (-0.04%) to 91.339% when pulling 210a26a on aeroastro:feature/tempfile-directory into 98f4f2e on rubyzip:master.

@aeroastro aeroastro force-pushed the aeroastro:feature/tempfile-directory branch from 210a26a to 258ef02 Jun 27, 2017
@coveralls
Copy link

@coveralls coveralls commented Jun 27, 2017

Coverage Status

Coverage decreased (-0.04%) to 91.339% when pulling 258ef02 on aeroastro:feature/tempfile-directory into 98f4f2e on rubyzip:master.

2 similar comments
@coveralls
Copy link

@coveralls coveralls commented Jun 27, 2017

Coverage Status

Coverage decreased (-0.04%) to 91.339% when pulling 258ef02 on aeroastro:feature/tempfile-directory into 98f4f2e on rubyzip:master.

@coveralls
Copy link

@coveralls coveralls commented Jun 27, 2017

Coverage Status

Coverage decreased (-0.04%) to 91.339% when pulling 258ef02 on aeroastro:feature/tempfile-directory into 98f4f2e on rubyzip:master.

@aeroastro
Copy link
Contributor Author

@aeroastro aeroastro commented Jun 27, 2017

Should I maintain or increase coverage for this patch to be merged?

Since this patch implements correct (or recommended) arguments to Ruby built-in library, IMHO there seems to be no strong requirements to keep the coverage.

Also, the fact the coverage decrease from https://coveralls.io/builds/10052092 (recent master push) to https://coveralls.io/builds/12143274 (this branch) is due to increased number of relevant lines from 2031 to 2032. This is actually triggered by counting end line as relevant lines here https://coveralls.io/builds/12143274/source?filename=lib%2Fzip%2Fstreamable_stream.rb#L9. It seems like there are some issues on coverage calculation side.

I just would like to stop temporary files from filling up application's root directory. 🐱

@aeroastro
Copy link
Contributor Author

@aeroastro aeroastro commented Nov 3, 2017

@simonoff

Hello.

It's been a while since I submitted this PR, and I'm wondering if there is anything I could do on my end. I totally understand you are busy and require more time to consider, it is much appreciated if you could give me a response.

@simonoff simonoff merged commit 57c3772 into rubyzip:master Nov 8, 2017
1 of 2 checks passed
1 of 2 checks passed
coverage/coveralls Coverage decreased (-0.04%) to 91.339%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@aeroastro aeroastro deleted the aeroastro:feature/tempfile-directory branch Nov 8, 2017
@aeroastro
Copy link
Contributor Author

@aeroastro aeroastro commented Nov 8, 2017

Thank you very much!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants