Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Define bank_snapshots_dir as tmpdir #31216

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

xiangzhu70
Copy link
Contributor

@xiangzhu70 xiangzhu70 commented Apr 16, 2023

Problem

As pointed in #31191 (comment), the bank_snapshots_dir can be a TempDir directly. So some old code doing that wrong need to be cleaned up.

Summary of Changes

Let bank_snapshots_dir be defined as TempDir.

Fixes #

@xiangzhu70 xiangzhu70 changed the title Bank snapshots dir tmpdir Declare bank_snapshots_dir as tmpdir Apr 16, 2023
@xiangzhu70 xiangzhu70 changed the title Declare bank_snapshots_dir as tmpdir Define bank_snapshots_dir as tmpdir Apr 16, 2023
@codecov
Copy link

codecov bot commented Apr 16, 2023

Codecov Report

Merging #31216 (ccda700) into master (07e038b) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #31216   +/-   ##
=======================================
  Coverage    81.5%    81.5%           
=======================================
  Files         729      729           
  Lines      206332   206328    -4     
=======================================
+ Hits       168277   168282    +5     
+ Misses      38055    38046    -9     

@xiangzhu70 xiangzhu70 marked this pull request as ready for review April 16, 2023 18:02
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

lgtm

@xiangzhu70 xiangzhu70 merged commit f066ea9 into solana-labs:master Apr 17, 2023
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Apr 17, 2023
* Let bank_snapshots_dir be TempDir

* Same change in test_bank_from_latest_snapshot_dir

* Use borrow to avoid releasing the tmp dir

* One more to change to reference
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants