-
Notifications
You must be signed in to change notification settings - Fork 795
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can also bump svm in cargo.lock file
we don't commit the lockfile ser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failing install test, not sure what's going on there
Some(dir) => { | ||
if !dir.exists() { | ||
dirs::data_dir().map(|dir| dir.join("svm")) | ||
} else { | ||
Some(dir) | ||
} | ||
} | ||
None => dirs::data_dir().map(|dir| dir.join("svm")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this also just SVM_DATA_DIR now?
can we use this instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the problem with using this directly is that we'd lock the crate to use svm always instead of conditionally (it's currently a feature)
@mattsse done, but current failure seems unrelated? |
yeah unrelated geth snafu @DaniPopes mind merging this wen you find the time? |
* fix: use SVM_DATA_DIR instead of SVM_HOME * chore: update test * chore: use correct folder if exists * fix: change svm home folder detection * clippy * actually check if path exists or not
Motivation
Due to alloy-rs/svm-rs@9b0fa3a#diff-4ac4c674e2696c0c39e91b7ba5af3f65b6ffb1a0fa9b90d3f9930a14254ecb98L126 some changes are needed to point to the correct var here, as
SVM_HOME
does not exist anymoreSolution
Use
SVM_DATA_DIR
PR Checklist