Alewis/publish multiple routes #922

ashleymichal · 2019-12-03T19:08:08Z

this pr gets us to where we actually publish all specified routes within a wrangler toml.

todo:
unit tests
at least one integration test
improve output to show how routes were updated/maintained.

src/commands/publish/mod.rs

src/commands/publish/route.rs

ashleymichal · 2019-12-03T23:31:03Z

src/commands/publish/mod.rs

-            if !route.ends_with("*") {
-                message::warn(&format!("The route in your wrangler.toml should have a trailing * to apply the Worker on every path, otherwise your site will not behave as expected.\nroute = {}*", route));
-            }
-        }


i moved this down into the deploy routes function, since it's only relevant to that path

ashleymichal · 2019-12-03T23:31:58Z

src/commands/publish/mod.rs

+    let routes = target.routes()?;
+    log::info!("routes: {:#?}", &routes);
+
+    if routes.is_empty() {


i'm nearly inclined to move this all just into the top level command function, since it doesn't really return anything, just does the console output.

i think i'd rather it be its own function than in the top level

i guess what i meant was, all the std out output and switching on routes etc. but i also think i might futz with this so i'm into it staying here

ashleymichal · 2019-12-03T23:33:10Z

src/commands/publish/mod.rs

@@ -132,25 +151,6 @@ fn error_msg(status: reqwest::StatusCode, text: String) -> String {
    }
 }

-#[test]


nothing about this test changed, i just moved it to the bottom of the module.

ashleymichal · 2019-12-03T23:35:14Z

src/commands/publish/mod.rs

@@ -258,7 +258,7 @@ fn validate_target_required_fields_present(target: &Target) -> Result<(), failur
        None => {}
    }

-    let destination = if target.route.is_some() {
+    let destination = if let Some(route) = &target.route {


i could fix this to check if target.routes()?.is_empty(), however i'm also inclined to shift some of this logic into something like a Manifest::get_deploy_config function instead, so we can keep all the logic around what is needed to publish in one place; right now it is spread across this module and settings::target

* include (optional) id in Route struct * remove unused Route::new() method * include upload result information in output of routes deploy function * split upload script and deploy into two functions

ashleymichal added changelog - feature regression Something is broken, but works in previous releases labels Dec 3, 2019

ashleymichal added this to the Multi Route Support milestone Dec 3, 2019

ashleymichal requested a review from a team December 3, 2019 19:08

ashleymichal self-assigned this Dec 3, 2019