Extract magic numbers, replace eprintln, rename VaultManager::lock to seal#96
Extract magic numbers, replace eprintln, rename VaultManager::lock to seal#96
Conversation
…::lock to seal - Add SSH_DEFAULT_PORT, DEFAULT_PTY_COLS, DEFAULT_PTY_ROWS constants to conch_remote and replace magic numbers in handler.rs and ssh.rs - Add missing MENU_TOGGLE_BOTTOM_PANEL_ID, MENU_ACTION_TOGGLE_BOTTOM_PANEL, MENU_ACTION_CHECK_UPDATES, and MENU_ACTION_ABOUT constants to menu.rs; replace inline string literals in menu.rs and lib.rs - Replace eprintln! with log::debug!/log::warn! in jvm/runtime.rs (keep eprintln in main.rs and test code) - Rename VaultManager::lock() to seal() to avoid confusion with Mutex::lock(); update call sites in vault_commands.rs and tests
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e502c536e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// | ||
| /// Named `seal` (rather than `lock`) to avoid confusion with Mutex::lock | ||
| /// on the `Arc<Mutex<VaultManager>>` wrapper used by the app. | ||
| pub fn seal(&self) { |
There was a problem hiding this comment.
lock API until all call sites are migrated
Renaming VaultManager::lock to seal here introduces a compile break because not all callers were updated: crates/conch_tauri/src/remote/ssh_commands.rs:655 still invokes mgr.lock(). Any test build that compiles this module (e.g. cargo test -p conch_tauri or workspace tests) will fail with no method named \lock``, so this refactor is currently incomplete unless you also migrate remaining usages or keep a temporary compatibility wrapper.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Three small cleanup items bundled:
1. Magic numbers → constants:
SSH_DEFAULT_PORT(22),DEFAULT_PTY_COLS(80),DEFAULT_PTY_ROWS(24) in conch_remoteMENU_ACTION_TOGGLE_BOTTOM_PANEL,MENU_ACTION_CHECK_UPDATES,MENU_ACTION_ABOUTin conch_tauri menu.rs2. eprintln → log:
eprintln!calls injvm/runtime.rsreplaced withlog::debug!/log::warn!3. VaultManager::lock() → seal():
vault.lock().lock()patternvault.lock().seal()— unambiguousType of change
Testing