-
Notifications
You must be signed in to change notification settings - Fork 258
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
workspace-support #599
workspace-support #599
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.
Wow, that was easy. Thanks @gautamprikshit1
Just confirm this is updating the correct function. The easiest would be to create a workspace with two crates:
- A crate with a library function like
get_name()
- A crate that uses shuttle and crate 1 to get a name
Then from inside crate 1 run cargo shuttle run
. If it fails then the wrong function might have been updated here
@@ -669,7 +670,12 @@ pub fn cargo_shuttle_init(path: PathBuf, framework: Framework) -> Result<()> { | |||
let mut dependencies = Table::new(); | |||
|
|||
// Set "shuttle-service" version to `[dependencies]` table | |||
let manifest_path = find(Some(path.as_path())).unwrap(); | |||
let config = cargo::util::config::Config::default().unwrap(); |
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.
Nice!
I'm just not sure if cargo_shuttle_init
is the correct function to update though. I think this function needs to be updated or removed (which is better)
shuttle/cargo-shuttle/src/lib.rs
Lines 210 to 214 in 79ff57e
fn find_root_directory(dir: &Path) -> Option<PathBuf> { | |
dir.ancestors() | |
.find(|ancestor| ancestor.join("Cargo.toml").exists()) | |
.map(|path| path.to_path_buf()) | |
} |
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.
Have you run it already or shall I do it??
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.
https://docs.rs/project-root/latest/src/project_root/lib.rs.html#19-34
I think replacing existing function with this one will do.
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.
I'll leave the testing to you 😄
Made an attempt at #564 please check