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

[MINOR] Delete temp file from docs #836

Merged
merged 3 commits into from
Oct 18, 2021
Merged

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Oct 18, 2021

Which issue does this PR close?

Closes #835

Rationale for this change

It is annoying that a file is left around after running cargo test

Prior to this PR, when you run cargo test --all the following file is present:

alamb@MacBook-Pro arrow-rs % git status
git status
On branch alamb/fix-publish
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	parquet/data.parquet

nothing added to commit but untracked files present (use "git add" to track)

What changes are included in this PR?

Remove file in doc tests

Are there any user-facing changes?

No

@alamb alamb added the development-process Related to development process of arrow-rs label Oct 18, 2021
@github-actions github-actions bot added the parquet Changes to the parquet crate label Oct 18, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2021

Codecov Report

Merging #836 (72ff56e) into master (491e564) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #836   +/-   ##
=======================================
  Coverage   82.60%   82.60%           
=======================================
  Files         168      168           
  Lines       48040    48040           
=======================================
  Hits        39685    39685           
  Misses       8355     8355           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 491e564...72ff56e. Read the comment docs.

Copy link
Member

@jimexist jimexist left a comment

Choose a reason for hiding this comment

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

you can consider adding it to the gitignore or adopt tempfile pattern

Copy link
Contributor Author

@alamb alamb left a comment

Choose a reason for hiding this comment

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

While this is likely not the cleanest solution available I think it is simple and fixes the problem so I am going to merge it in.

If anyone else has a chance, cleaning it up to @jimexist 's suggestion (e.g. use tempfile) that would be great 🙏

@alamb
Copy link
Contributor Author

alamb commented Oct 18, 2021

🤔 well the test fails on https://github.com/apache/arrow-rs/pull/836/checks?check_run_id=3930315965 and rather than spending time debugging it I am going to take @jimexist 's advice and use .gitignore instead

@alamb alamb merged commit 7472e5e into apache:master Oct 18, 2021
@alamb alamb deleted the alamb/fix_file branch October 24, 2021 10:40
alamb added a commit that referenced this pull request Oct 24, 2021
* Delete temp file from docs

* fix

* Use gitignore instead
alamb added a commit that referenced this pull request Oct 24, 2021
* Delete temp file from docs

* fix

* Use gitignore instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development-process Related to development process of arrow-rs parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo test --all leaves data.parquet around
3 participants