Updates from review

update
vercel · Apr 20, 2023 · a4a6596 · a4a6596
1 parent 9eb2c74
commit a4a6596
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 49 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/turborepo-lib/src/cli.rs b/crates/turborepo-lib/src/cli.rs
@@ -258,8 +258,8 @@ pub enum Command {
         no_gitignore: bool,
 
         /// Specify what should be linked (default "remote cache")
-        #[clap(long, value_enum)]
-        target: Option<LinkTarget>,
+        #[clap(long, value_enum, default_value_t = LinkTarget::RemoteCache)]
+        target: LinkTarget,
     },
     /// Login to your Vercel account
     Login {
@@ -292,8 +292,8 @@ pub enum Command {
     /// Remote Caching
     Unlink {
         /// Specify what should be unlinked (default "remote cache")
-        #[clap(long, value_enum)]
-        target: Option<LinkTarget>,
+        #[clap(long, value_enum, default_value_t = LinkTarget::RemoteCache)]
+        target: LinkTarget,
     },
 }
 
@@ -1228,7 +1228,9 @@ mod test {
         assert_eq!(
             Args::try_parse_from(["turbo", "unlink"]).unwrap(),
             Args {
-                command: Some(Command::Unlink { target: None }),
+                command: Some(Command::Unlink {
+                    target: crate::cli::LinkTarget::RemoteCache
+                }),
                 ..Args::default()
             }
         );
@@ -1238,7 +1240,9 @@ mod test {
             command_args: vec![],
             global_args: vec![vec!["--cwd", "../examples/with-yarn"]],
             expected_output: Args {
-                command: Some(Command::Unlink { target: None }),
+                command: Some(Command::Unlink {
+                    target: crate::cli::LinkTarget::RemoteCache,
+                }),
                 cwd: Some(PathBuf::from("../examples/with-yarn")),
                 ..Args::default()
             },

diff --git a/crates/turborepo-lib/src/commands/link.rs b/crates/turborepo-lib/src/commands/link.rs
@@ -111,10 +111,8 @@ pub(crate) async fn verify_caching_enabled<'a>(
 pub async fn link(
     base: &mut CommandBase,
     modify_gitignore: bool,
-    target: Option<LinkTarget>,
+    target: LinkTarget,
 ) -> Result<()> {
-    let target = target.unwrap_or(LinkTarget::RemoteCache);
-
     let homedir_path = home_dir().ok_or_else(|| anyhow!("could not find home directory."))?;
     let homedir = homedir_path.to_string_lossy();
     let repo_root_with_tilde = base.repo_root.to_string_lossy().replacen(&*homedir, "~", 1);
@@ -219,28 +217,28 @@ pub async fn link(
                 SelectedSpace::Space(space) => space,
             };
 
-            let persisted_space_id = add_space_id_to_turbo_json(base, &space.id);
+            add_space_id_to_turbo_json(base, &space.id).map_err(|err| {
+                return anyhow!(
+                    "Could not persist selected space ({}) to `experimentalSpaces.id` in \
+                     turbo.json {}",
+                    space.id,
+                    err
+                );
+            })?;
 
-            match persisted_space_id {
-                Ok(_) => {
-                    println!(
-                        "
+            println!(
+                "
     {} {} linked to {}
 
     {}
         ",
-                        base.ui.rainbow(">>> Success!"),
-                        base.ui.apply(BOLD.apply_to(&repo_root_with_tilde)),
-                        base.ui.apply(BOLD.apply_to(&space.name)),
-                        GREY.apply_to(
-                            "To remove Spaces integration, run `npx turbo unlink --target spaces`"
-                        )
-                    );
-                }
-                Err(e) => {
-                    return Err(anyhow!("Could not persist space id to turbo.json: {}", e));
-                }
-            }
+                base.ui.rainbow(">>> Success!"),
+                base.ui.apply(BOLD.apply_to(&repo_root_with_tilde)),
+                base.ui.apply(BOLD.apply_to(&space.name)),
+                GREY.apply_to(
+                    "To remove Spaces integration, run `npx turbo unlink --target spaces`"
+                )
+            );
 
             return Ok(());
         }
@@ -501,7 +499,7 @@ mod test {
             version: "",
         };
 
-        link::link(&mut base, false, Option::Some(LinkTarget::RemoteCache))
+        link::link(&mut base, false, LinkTarget::RemoteCache)
             .await
             .unwrap();
 
@@ -558,7 +556,7 @@ mod test {
         )
         .unwrap();
 
-        link::link(&mut base, false, Option::Some(LinkTarget::Spaces))
+        link::link(&mut base, false, LinkTarget::Spaces)
             .await
             .unwrap();
 

diff --git a/crates/turborepo-lib/src/commands/unlink.rs b/crates/turborepo-lib/src/commands/unlink.rs
@@ -2,12 +2,7 @@ use std::fs::File;
 
 use anyhow::{anyhow, Context, Result};
 
-use crate::{
-    cli::LinkTarget,
-    commands::CommandBase,
-    config::{SpacesJson, TurboJson},
-    ui::GREY,
-};
+use crate::{cli::LinkTarget, commands::CommandBase, config::TurboJson, ui::GREY};
 
 enum UnlinkSpacesResult {
     Unlinked,
@@ -45,9 +40,7 @@ fn unlink_spaces(base: &mut CommandBase) -> Result<()> {
     Ok(())
 }
 
-pub fn unlink(base: &mut CommandBase, target: Option<LinkTarget>) -> Result<()> {
-    let target = target.unwrap_or(LinkTarget::RemoteCache);
-
+pub fn unlink(base: &mut CommandBase, target: LinkTarget) -> Result<()> {
     match target {
         LinkTarget::RemoteCache => {
             unlink_remote_caching(base)?;
@@ -62,18 +55,11 @@ pub fn unlink(base: &mut CommandBase, target: Option<LinkTarget>) -> Result<()>
 fn remove_spaces_from_turbo_json(base: &CommandBase) -> Result<UnlinkSpacesResult> {
     let turbo_json_path = base.repo_root.join("turbo.json");
 
-    if !turbo_json_path.exists() {
-        return Err(anyhow!("turbo.json not found."));
-    }
-
-    let turbo_json_file = File::open(&turbo_json_path)?;
+    let turbo_json_file = File::open(&turbo_json_path).context("unable to open turbo.json file")?;
     let mut turbo_json: TurboJson = serde_json::from_reader(turbo_json_file)?;
     let has_spaces_id = turbo_json
         .experimental_spaces
-        .unwrap_or(SpacesJson {
-            id: None,
-            other: None,
-        })
+        .unwrap_or_default()
         .id
         .is_some();
     // remove the spaces config

diff --git a/crates/turborepo-lib/src/config/turbo.rs b/crates/turborepo-lib/src/config/turbo.rs
@@ -1,6 +1,6 @@
 use serde::{Deserialize, Serialize};
 
-#[derive(Serialize, Deserialize, Debug)]
+#[derive(Serialize, Deserialize, Debug, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct SpacesJson {
     pub id: Option<String>,