Skip to content

Commit b7d3284

Browse files
sachiniyerclaude
andauthored
refactor(config): remove dead save_config function (#287)
## Summary - Deletes the unused `save_config` function in `src/config/storage.rs` and its dedicated `save_then_load_config` round-trip test. - Updates `concurrent_update_config_does_not_corrupt` to seed the config file with `update_config(|_| {})` instead of `save_config(&Config::default())`. - Closes Detail bug `bug_3b5a0cd5-08da-482d-b222-ce83e5acb944` (linked GH #269), which reported a truncate-before-lock race in `save_config`. Fixing the race isn't worth it since the function has no production callers — `grep -rn save_config --include='*.rs'` showed only the function definition and two `#[cfg(test)]` sites in the same file. ## Note for reviewers `save_config` is `pub`, and although `Cargo.toml` only declares a `[[bin]]`, the crate also has `src/lib.rs`, so Cargo auto-builds a library and `detail_cli::config::storage::save_config` was technically part of the library's public API. We don't publish to crates.io and there are no other consumers of this crate's library surface that we know of, but flagging in case you want to weigh that. ## Test plan - [x] `cargo test` — all 289 lib + 17 integration tests pass - [x] `grep -rn save_config --include='*.rs'` returns no matches after the change - [x] `concurrent_update_config_does_not_corrupt` still passes with the new init line 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f5db2ea commit b7d3284

1 file changed

Lines changed: 2 additions & 29 deletions

File tree

‎src/config/storage.rs‎

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,6 @@ pub fn load_config() -> Result<Config> {
6464
toml::from_str(&contents).context("Failed to parse config")
6565
}
6666

67-
pub fn save_config(config: &Config) -> Result<()> {
68-
let path = config_path()?;
69-
let contents = toml::to_string_pretty(config)?;
70-
let file = File::create(&path)?;
71-
file.lock_exclusive()?;
72-
(&file).write_all(contents.as_bytes())?;
73-
file.unlock()?;
74-
Ok(())
75-
}
76-
7767
/// Atomically read-modify-write the config file under an exclusive lock.
7868
///
7969
/// Preserves comments and formatting the user may have added by hand. The
@@ -262,24 +252,7 @@ mod tests {
262252
});
263253
}
264254

265-
// ── save / load round-trip ───────────────────────────────────────
266-
267-
#[test]
268-
fn save_then_load_config() {
269-
with_temp_config(|| {
270-
let config = Config {
271-
api_url: Some("https://test.dev".into()),
272-
app_url: None,
273-
check_for_updates: true,
274-
last_update_check: None,
275-
api_token: Some("tok".into()),
276-
};
277-
save_config(&config).unwrap();
278-
let loaded = load_config().unwrap();
279-
assert_eq!(loaded.api_url.as_deref(), Some("https://test.dev"));
280-
assert_eq!(loaded.api_token.as_deref(), Some("tok"));
281-
});
282-
}
255+
// ── load_config ──────────────────────────────────────────────────
283256

284257
#[test]
285258
fn load_config_partial_file_defaults_check_for_updates() {
@@ -492,7 +465,7 @@ api_token = \"old_token\"
492465

493466
with_temp_config(|| {
494467
// Initialize config
495-
save_config(&Config::default()).unwrap();
468+
update_config(|_| {}).unwrap();
496469

497470
let handles: Vec<_> = (0..10)
498471
.map(|i| {

0 commit comments

Comments
 (0)