Refactor: Init function cleanup #705
Conversation
|
Let me know if there's anything you need me to update or do with this @oddgrd . I know a few things have changed since it was requested so if I can help ease the load with anything just let me know. |
cargo-shuttle/src/cargo_builder.rs
Outdated
| match self.dependencies.get_mut(&x.name) { | ||
| Some(y) => { | ||
| y.insert(attribute_name, dep_value); | ||
| } | ||
| None => { | ||
| self.dependencies.insert( | ||
| x.name.to_owned(), | ||
| BTreeMap::from([(attribute_name, dep_value)]), | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| self.dependencies | ||
| .get_mut(&x.name) | ||
| .unwrap() | ||
| .insert("version".to_owned(), Value::from(x.get_latest_version())); |
There was a problem hiding this comment.
| match self.dependencies.get_mut(&x.name) { | |
| Some(y) => { | |
| y.insert(attribute_name, dep_value); | |
| } | |
| None => { | |
| self.dependencies.insert( | |
| x.name.to_owned(), | |
| BTreeMap::from([(attribute_name, dep_value)]), | |
| ); | |
| } | |
| }; | |
| self.dependencies | |
| .get_mut(&x.name) | |
| .unwrap() | |
| .insert("version".to_owned(), Value::from(x.get_latest_version())); | |
| let mut dependency = self.dependencies.entry(&x.name).or_insert(BTreeMap::new()); | |
| dependency.insert(attribute_name, dep_value); | |
| dependency.insert("version".to_owned(), Value::from(x.get_latest_version())); |
The entry(..).or_insert(..) makes it easier to have a default entry to work with.
There was a problem hiding this comment.
I might also be wrong, but I think this add a version the second time - ie add_dependency adds a version too in the attributes
| /// values without a version will be calculated automatically | ||
| pub fn add_var( | ||
| &mut self, | ||
| section: CargoSection, |
There was a problem hiding this comment.
I think we can drop the section enum by moving the match block to the respective functions below.
There was a problem hiding this comment.
Did you look into this @jmwill86?
cargo-shuttle/src/init.rs
Outdated
| fn get_base_dependencies(&self) -> Vec<&str>; | ||
| fn get_dependency_attributes(&self) -> HashMap<&str, HashMap<&str, Value>>; | ||
| fn get_boilerplate_code_for_framework(&self) -> &'static str; |
|
I've merged existing changes into this branch, it was a little fiddly but I think I've transferred everything over successfully. I'm not sure if the differences in the examples directory are something I've done and need to correct, or if this is unrelated. I'm not too familiar with git submodules but point me in the right direction to correct this and I'll get it fixed. |
| /// values without a version will be calculated automatically | ||
| pub fn add_var( | ||
| &mut self, | ||
| section: CargoSection, |
There was a problem hiding this comment.
Did you look into this @jmwill86?
cargo-shuttle/src/init.rs
Outdated
| } | ||
|
|
||
| fn get_boilerplate_code_for_framework(&self) -> &'static str { | ||
| indoc! {r#" | ||
| use axum::{routing::get, Router}; | ||
|
|
There was a problem hiding this comment.
This is a bit of a nitpick, but shouldn't we have newlines here?
cargo-shuttle/src/init.rs
Outdated
| fn get_base_dependencies(&self) -> Vec<&str> { | ||
| vec!["rocket", "shuttle-rocket", "tokio", "shuttle-runtime"] | ||
| } |
There was a problem hiding this comment.
This is so much simpler, nice!
cargo-shuttle/src/cargo_builder.rs
Outdated
| crate_ver | ||
| .highest_normal_version() | ||
| .unwrap() | ||
| .version() | ||
| .to_string() |
There was a problem hiding this comment.
Are we still covering the case where we want to fetch a pre-release version? For example for rocket we want init to start with rocket 0.5rc-3, not the latest stable release which is 0.4.x.
Adding a new framework to Shuttle currently takes a lot of duplicating existing code, and that code is defined in various files making it very error prone. I see this commit as step 1 or 2 (the second being a refactor of cargo-shuttle/args.rs) to making the process easier and cleaner for people wanting to add a new framework.
This commit includes a clean up of the cargo-shuttle init.rs functionality with an addition of a cargo builder to make any future Cargo.toml changes easier. This is largely separation of concerns and cutting out duplicated code to improve usability and testability.
Testing has remained largely the same or improved from the original.
I've added a few
#[allow(dead_code)]elements in there too, the reason is that although a couple of functions aren't used at the moment I think they may be useful in future. There may be a better way to ignore these warnings.