refactor: extract _save_config_safely helper in TTS window - #61
Merged
Merged
Conversation
Replace six identical try/except/pass config-save blocks in TtsWindow with a module-level helper next to _safe_unlink(). The helper is a module function rather than a method because tests call TtsWindow._ensure_openai_consent and _revert_provider_to_piper unbound with SimpleNamespace fakes. _on_speed_changed is left as is: its try also covers the tts_speed property setter, which can raise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
4 tasks
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
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.
What changed?
Adds a module-level
_save_config_safely(config)helper inapp/tts_window.py(next to_safe_unlink()) and replaces six identicaltry: config.save() / except Exception: passblocks with it:_ensure_openai_consent,_revert_provider_to_piper,_on_provider_changed,_on_voice_changed(2x),_on_model_changed. One file, +14/-24, no behavior change._on_speed_changedis intentionally left as is: itstryalso covers thetts_speedproperty setter (float(value)), which can raise.Why?
Removes duplicated silent error handling from a cleanup audit (ruff/vulture/jscpd). It is a module function, not a method, because the tests call
TtsWindow._ensure_openai_consentand_revert_provider_to_piperunbound withSimpleNamespacefakes. No logging was added on purpose; that is a separate step.How did you test it?
Baseline on
mainvs. this branch,QT_QPA_PLATFORM=offscreen WHISPER_GUI_TESTS=1:pytest tests/app/tts_window.py(0.16.8, default rules)git diff --checkis clean.AI assistance
Yes. The refactor was reconstructed and verified with Claude Code (Claude Sonnet 5) from an earlier audit; the diff was reviewed before committing.
Checklist
pytest tests/(offscreen) or explained why not.🤖 Generated with Claude Code