Repository: axodotdev/cargo-dist
Pull request #392 (2023-09-01 16:31:39)

feat(ci): cleanup default/init logic for pr_run_mode
* make plan the default (as originally agreed?)
* make the init code actually remember what you had set before
* make the init code not write a value if it's the default
* make the init code produce compile errors if a new PrRunMode is added
* rework the prompt order/messages to be clearer


Patch:
diff --git a/cargo-dist-schema/src/lib.rs b/cargo-dist-schema/src/lib.rs
index 68624307..c4825d12 100644
--- a/cargo-dist-schema/src/lib.rs
+++ b/cargo-dist-schema/src/lib.rs
@@ -119,17 +119,18 @@ pub struct GithubMatrixEntry {
 }
 
 /// Type of job to run on pull request
-#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, Default)]
-#[non_exhaustive]
+#[derive(
+    Debug, Copy, Clone, Serialize, Deserialize, JsonSchema, Default, PartialEq, Eq, PartialOrd, Ord,
+)]
 pub enum PrRunMode {
     /// Do not run on pull requests at all
     #[serde(rename = "skip")]
     Skip,
     /// Only run the plan step
+    #[default]
     #[serde(rename = "plan")]
     Plan,
     /// Build and upload artifacts
-    #[default]
     #[serde(rename = "upload")]
     Upload,
 }
diff --git a/cargo-dist/src/backend/ci/github.rs b/cargo-dist/src/backend/ci/github.rs
index ffbed8b3..09ac3de8 100644
--- a/cargo-dist/src/backend/ci/github.rs
+++ b/cargo-dist/src/backend/ci/github.rs
@@ -91,7 +91,7 @@ impl GithubCiInfo {
             None
         };
 
-        let pr_run_mode = dist.pr_run_mode.clone();
+        let pr_run_mode = dist.pr_run_mode;
         let allow_dirty = dist.allow_dirty.contains(&CiStyle::Github);
 
         let tap = dist.tap.clone();
diff --git a/cargo-dist/src/init.rs b/cargo-dist/src/init.rs
index be7c927f..2e61e6c5 100644
--- a/cargo-dist/src/init.rs
+++ b/cargo-dist/src/init.rs
@@ -386,27 +386,41 @@ fn get_new_dist_metadata(
     }
 
     if has_github_ci {
-        let prompt = r#"enable Github CI checks on pull requests?
-    This will run most of your release process in order to allow you to
-    test it regularly. "Upload" will build and upload the release artifacts,
-    while "plan" will only plan out the release without actually
-    performing it."#;
-        let items = vec!["upload", "plan", "skip"];
+        let default_val = PrRunMode::default();
+        let cur_val = meta.pr_run_mode.unwrap_or(default_val);
+
+        // This is intentionally written awkwardly to make you update this!
+        //
+        // don't forget to add it to 'items' below!
+        let desc = |val| match val {
+            PrRunMode::Skip => "skip - don't check the release process in PRs",
+            PrRunMode::Plan => "plan - run 'cargo dist plan' on PRs (recommended)",
+            PrRunMode::Upload => "upload - build and upload an artifacts.zip to the PR (expensive)",
+        };
+        let items = [PrRunMode::Skip, PrRunMode::Plan, PrRunMode::Upload];
+
+        // Get the index of the current value
+        let default = items
+            .iter()
+            .position(|val| val == &cur_val)
+            .expect("someone added a pr_run_mode but forgot to add it to 'init'");
+
+        let prompt = r#"check your release process in pull requests?"#;
         let selection = Select::with_theme(&theme)
             .with_prompt(prompt)
-            .items(&items)
-            .default(0)
+            .items(&items.iter().map(|mode| desc(*mode)).collect::<Vec<_>>())
+            .default(default)
             .interact()?;
         eprintln!();
 
-        let result = match items[selection] {
-            "upload" => PrRunMode::Upload,
-            "plan" => PrRunMode::Plan,
-            "skip" => PrRunMode::Skip,
-            _ => PrRunMode::default(),
-        };
+        let result = items[selection];
 
-        meta.pr_run_mode = Some(result);
+        // Don't add needlessly noisy config if it's just the default
+        meta.pr_run_mode = if result == default_val {
+            None
+        } else {
+            Some(result)
+        };
     }
 
     // Enable installer backends (if they have a CI backend that can provide URLs)
diff --git a/cargo-dist/src/lib.rs b/cargo-dist/src/lib.rs
index b0609483..c381ee6a 100644
--- a/cargo-dist/src/lib.rs
+++ b/cargo-dist/src/lib.rs
@@ -148,7 +148,7 @@ fn build_manifest(cfg: &Config, dist: &DistGraph) -> DistManifest {
         let CiInfo { github } = &dist.ci;
         let github = github.as_ref().map(|info| cargo_dist_schema::GithubCiInfo {
             artifacts_matrix: Some(info.artifacts_matrix.clone()),
-            pr_run_mode: Some(info.pr_run_mode.clone()),
+            pr_run_mode: Some(info.pr_run_mode),
             allow_dirty: Some(info.allow_dirty),
         });
 
diff --git a/cargo-dist/src/tasks.rs b/cargo-dist/src/tasks.rs
index 3e87ec77..abb801c6 100644
--- a/cargo-dist/src/tasks.rs
+++ b/cargo-dist/src/tasks.rs
@@ -720,7 +720,7 @@ impl<'pkg_graph> DistGraphBuilder<'pkg_graph> {
                 variants: vec![],
                 releases: vec![],
                 ci: CiInfo::default(),
-                pr_run_mode: workspace_metadata.pr_run_mode.clone().unwrap_or_default(),
+                pr_run_mode: workspace_metadata.pr_run_mode.unwrap_or_default(),
                 tap: workspace_metadata.tap.clone(),
                 publish_jobs,
                 allow_dirty,


Test patch:
diff --git a/cargo-dist/tests/snapshots/akaikatana_basic.snap b/cargo-dist/tests/snapshots/akaikatana_basic.snap
index 07949ad6..82953ba5 100644
--- a/cargo-dist/tests/snapshots/akaikatana_basic.snap
+++ b/cargo-dist/tests/snapshots/akaikatana_basic.snap
@@ -1262,7 +1262,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/akaikatana_repo_with_dot_git.snap b/cargo-dist/tests/snapshots/akaikatana_repo_with_dot_git.snap
index 07949ad6..82953ba5 100644
--- a/cargo-dist/tests/snapshots/akaikatana_repo_with_dot_git.snap
+++ b/cargo-dist/tests/snapshots/akaikatana_repo_with_dot_git.snap
@@ -1262,7 +1262,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/axolotlsay_basic.snap b/cargo-dist/tests/snapshots/axolotlsay_basic.snap
index f2f144a6..8c899a93 100644
--- a/cargo-dist/tests/snapshots/axolotlsay_basic.snap
+++ b/cargo-dist/tests/snapshots/axolotlsay_basic.snap
@@ -2135,7 +2135,7 @@ maybeInstall(true).then(run);
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/axolotlsay_edit_existing.snap b/cargo-dist/tests/snapshots/axolotlsay_edit_existing.snap
index b89ea96c..b448e24c 100644
--- a/cargo-dist/tests/snapshots/axolotlsay_edit_existing.snap
+++ b/cargo-dist/tests/snapshots/axolotlsay_edit_existing.snap
@@ -2135,7 +2135,7 @@ maybeInstall(true).then(run);
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/axolotlsay_no_homebrew_publish.snap b/cargo-dist/tests/snapshots/axolotlsay_no_homebrew_publish.snap
index 5e8b2cc6..b7154ba3 100644
--- a/cargo-dist/tests/snapshots/axolotlsay_no_homebrew_publish.snap
+++ b/cargo-dist/tests/snapshots/axolotlsay_no_homebrew_publish.snap
@@ -2135,7 +2135,7 @@ maybeInstall(true).then(run);
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_cargo_home.snap b/cargo-dist/tests/snapshots/install_path_cargo_home.snap
index 72264e7c..f9715816 100644
--- a/cargo-dist/tests/snapshots/install_path_cargo_home.snap
+++ b/cargo-dist/tests/snapshots/install_path_cargo_home.snap
@@ -1264,7 +1264,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_env_no_subdir.snap b/cargo-dist/tests/snapshots/install_path_env_no_subdir.snap
index e24b311a..4cf449ac 100644
--- a/cargo-dist/tests/snapshots/install_path_env_no_subdir.snap
+++ b/cargo-dist/tests/snapshots/install_path_env_no_subdir.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_env_subdir.snap b/cargo-dist/tests/snapshots/install_path_env_subdir.snap
index 102a7045..8b16fe63 100644
--- a/cargo-dist/tests/snapshots/install_path_env_subdir.snap
+++ b/cargo-dist/tests/snapshots/install_path_env_subdir.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_env_subdir_space.snap b/cargo-dist/tests/snapshots/install_path_env_subdir_space.snap
index c34a220c..d7ad87f4 100644
--- a/cargo-dist/tests/snapshots/install_path_env_subdir_space.snap
+++ b/cargo-dist/tests/snapshots/install_path_env_subdir_space.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_env_subdir_space_deeper.snap b/cargo-dist/tests/snapshots/install_path_env_subdir_space_deeper.snap
index 9a4cc8cd..4a043ba1 100644
--- a/cargo-dist/tests/snapshots/install_path_env_subdir_space_deeper.snap
+++ b/cargo-dist/tests/snapshots/install_path_env_subdir_space_deeper.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_home_subdir_deeper.snap b/cargo-dist/tests/snapshots/install_path_home_subdir_deeper.snap
index 2788db83..55ac4d40 100644
--- a/cargo-dist/tests/snapshots/install_path_home_subdir_deeper.snap
+++ b/cargo-dist/tests/snapshots/install_path_home_subdir_deeper.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_home_subdir_min.snap b/cargo-dist/tests/snapshots/install_path_home_subdir_min.snap
index 57e74296..543b4bce 100644
--- a/cargo-dist/tests/snapshots/install_path_home_subdir_min.snap
+++ b/cargo-dist/tests/snapshots/install_path_home_subdir_min.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_home_subdir_space.snap b/cargo-dist/tests/snapshots/install_path_home_subdir_space.snap
index e50cbc61..281b29c7 100644
--- a/cargo-dist/tests/snapshots/install_path_home_subdir_space.snap
+++ b/cargo-dist/tests/snapshots/install_path_home_subdir_space.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/install_path_home_subdir_space_deeper.snap b/cargo-dist/tests/snapshots/install_path_home_subdir_space_deeper.snap
index fd45d536..c9126dea 100644
--- a/cargo-dist/tests/snapshots/install_path_home_subdir_space_deeper.snap
+++ b/cargo-dist/tests/snapshots/install_path_home_subdir_space_deeper.snap
@@ -1244,7 +1244,7 @@ Install-Binary "$Args"
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": true
     }
   }
diff --git a/cargo-dist/tests/snapshots/lib_manifest.snap b/cargo-dist/tests/snapshots/lib_manifest.snap
index 55870125..168c9325 100644
--- a/cargo-dist/tests/snapshots/lib_manifest.snap
+++ b/cargo-dist/tests/snapshots/lib_manifest.snap
@@ -15,7 +15,7 @@ stdout:
   "ci": {
     "github": {
       "artifacts_matrix": {},
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": false
     }
   }
diff --git a/cargo-dist/tests/snapshots/lib_manifest_slash.snap b/cargo-dist/tests/snapshots/lib_manifest_slash.snap
index 1c6687cd..890873c0 100644
--- a/cargo-dist/tests/snapshots/lib_manifest_slash.snap
+++ b/cargo-dist/tests/snapshots/lib_manifest_slash.snap
@@ -15,7 +15,7 @@ stdout:
   "ci": {
     "github": {
       "artifacts_matrix": {},
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": false
     }
   }
diff --git a/cargo-dist/tests/snapshots/manifest.snap b/cargo-dist/tests/snapshots/manifest.snap
index 5900b506..c10f57e5 100644
--- a/cargo-dist/tests/snapshots/manifest.snap
+++ b/cargo-dist/tests/snapshots/manifest.snap
@@ -258,7 +258,7 @@ stdout:
           }
         ]
       },
-      "pr_run_mode": "upload",
+      "pr_run_mode": "plan",
       "allow_dirty": false
     }
   }