refactor(config): remove dead save_config function#287
Merged
sachiniyer merged 1 commit intomainfrom May 9, 2026
Merged
Conversation
Drop save_config and its dedicated round-trip test. The function had no production callers — the only references were two #[cfg(test)] sites in the same file. Closes Detail bug bug_3b5a0cd5 / GH issue #269 (truncate- before-lock race in save_config) by removing the function rather than fixing the race. The concurrent_update_config_does_not_corrupt test now seeds the config file with update_config(|_| {}) instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sachiniyer
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
save_configfunction insrc/config/storage.rsand its dedicatedsave_then_load_configround-trip test.concurrent_update_config_does_not_corruptto seed the config file withupdate_config(|_| {})instead ofsave_config(&Config::default()).bug_3b5a0cd5-08da-482d-b222-ce83e5acb944(linked GH [Detail Bug] Config writes can lose existing data due to truncation before file lock in save_config #269), which reported a truncate-before-lock race insave_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_configispub, and althoughCargo.tomlonly declares a[[bin]], the crate also hassrc/lib.rs, so Cargo auto-builds a library anddetail_cli::config::storage::save_configwas 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
cargo test— all 289 lib + 17 integration tests passgrep -rn save_config --include='*.rs'returns no matches after the changeconcurrent_update_config_does_not_corruptstill passes with the new init line🤖 Generated with Claude Code