Skip to content

feat(i18n): extract tools common approval messages - #861

Open
thomwebb wants to merge 4 commits into
mpfaffenberger:mainfrom
thomwebb:feat/i18n-extract-tools-common
Open

feat(i18n): extract tools common approval messages#861
thomwebb wants to merge 4 commits into
mpfaffenberger:mainfrom
thomwebb:feat/i18n-extract-tools-common

Conversation

@thomwebb

Copy link
Copy Markdown
Collaborator

Continues the CLI i18n extraction effort by localizing user-facing approval messages in code_puppy/tools/common.py.

  • Adds tools.common.approval.* catalog keys.
  • Adds Spanish and Canadian French translations.
  • Preserves dynamic placeholders and keeps blank lines/markup out of catalogs.
  • Adds focused catalog, pseudolocale, and placeholder tests.

@WSxDemise

Copy link
Copy Markdown
Collaborator

[Wes's CodePuppy Agent Review]

PR Summary

Localizes tools.common approval prompts and adds en-US, Spanish, and Canadian French catalog coverage.

Qodo Merge Triage

Qodo: not configured for OSS/public GitHub.

Code Review Findings

Really Should Fix These

  • [P1] Reuse the shared i18n isolation fixture — tests/i18n/test_common_i18n.py:14
    • Impact: A second autouse reset runs for every test and can drift from the shared isolation contract.
    • Fix: Delete _reset_locale; tests/i18n/conftest.py::_i18n_isolate already resets the locale and catalog.
  • [P1] Use centralized namespace health checks — tests/i18n/test_common_i18n.py:33
    • Impact: Duplicating generic resolution and pseudolocale checks lets this namespace drift from the centralized suite.
    • Fix: Add tools.common.approval. to _NAMESPACES and delete the duplicate generic health test.

Nits

None.

@thomwebb

Copy link
Copy Markdown
Collaborator Author

Addressed review feedback in commit 7398175f.

  • Removed the duplicate per-file locale reset fixture; the shared tests/i18n/conftest.py fixture now owns isolation.
  • Registered tools.common.approval. in the centralized namespace health suite.
  • Removed the duplicated generic resolution/pseudolocale test.
  • Re-ran Ruff formatting, compilation, and diff checks successfully.

@WSxDemise

Copy link
Copy Markdown
Collaborator

[Wes's CodePuppy Agent Review]

PR Summary

Localizes tools.common approval prompts and adds en-US, Spanish, and Canadian French catalog coverage.

Qodo Merge Triage

Qodo: not configured for OSS/public GitHub.

Code Review Findings

Really Should Fix These

  • [P1] Keep session namespace health checks — tests/i18n/test_catalog_namespaces.py:46
    • Impact: Replacing the session registration drops resolver, pseudolocale, and placeholder coverage for its catalog keys.
    • Fix: Retain the session entry and append tools.common.approval as a second _NAMESPACES entry.

Nits

None.

@thomwebb

Copy link
Copy Markdown
Collaborator Author

Updated in commit 5f093b65. Merged the current upstream main and resolved the locale catalog conflicts while preserving both upstream keys and this PR’s tools.common.approval.* entries. The branch is pushed and no additional review changes were requested on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants