Skip to content

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Oct 13, 2025

Issue being fixed or feature implemented

  • make strings easier to translate
  • stop translating non-translatable strings
  • use %n where possible

What was done?

How Has This Been Tested?

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

- make strings easier to translate
- stop translating non-translatable strings
- use `%n` where possible
@UdjinM6 UdjinM6 added this to the 23 milestone Oct 13, 2025
Copy link

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

Copy link

coderabbitai bot commented Oct 13, 2025

Walkthrough

This change updates UI and message strings to improve translation handling and simplifies result message assembly. In the UI .ui file, several static strings and placeholders are annotated with notr="true". In proposal creation, plural-aware formatting is used for ETA minutes and combo item text generation is adjusted. In governance voting, the result summary is refactored to linearly build a message that conditionally includes success counts, failure counts, and error details. No new UI elements, business logic, or public interfaces are introduced.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately and concisely describes the primary change of improving governance and proposal dialog strings, which aligns with the metadata attribute updates and pluralization enhancements introduced in the UI code.
Description Check ✅ Passed The pull request description directly addresses the translation improvements and non-translatable string adjustments that match the changes in the UI and Qt code, making it related and on-topic for this changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/qt/forms/proposalwizard.ui (1)

430-430: Consider whether "TxID:" should be translatable.

The label "TxID:" is marked with notr="true". While technical terms are often kept in English, some translation systems prefer to localize technical UI labels for consistency with the rest of the interface. This is a design decision.

If you decide "TxID:" should be translatable, remove the notr="true" attribute:

-<string notr="true">TxID:</string>
+<string>TxID:</string>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb01c5a and af85608.

📒 Files selected for processing (3)
  • src/qt/forms/proposalwizard.ui (7 hunks)
  • src/qt/governancelist.cpp (1 hunks)
  • src/qt/proposalwizard.cpp (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

  • src/qt/proposalwizard.cpp
  • src/qt/governancelist.cpp
🔇 Additional comments (7)
src/qt/proposalwizard.cpp (3)

63-63: LGTM! Numbers should not be translated.

The change from tr("%1").arg(i) to QString().setNum(i) correctly removes translation wrapping from numeric values, which should never be translated.


293-294: LGTM! Proper Qt pluralization.

The change to tr("Estimated time remaining: %n minute(s)", "", mins) correctly uses Qt's plural-aware formatting with the %n placeholder, enabling proper translations for different plural forms across languages.


324-325: LGTM! Correctly separates translatable from non-translatable text.

The change separates the translatable success message from the technical ID by concatenating tr("Your proposal was submitted successfully.") with QString("\nID: %1").arg(govId). This prevents the ID from being included in translation strings where it doesn't belong.

src/qt/forms/proposalwizard.ui (3)

113-113: LGTM! Example placeholders should not be translated.

Correctly marking placeholder text like "short-unique-name" and "https://example.com/my-proposal" with notr="true" as these are examples that should remain consistent across all languages.

Also applies to: 139-139


258-258: LGTM! Numbers should not be translated.

Correctly marking the numeric value "0" with notr="true" as numbers should not be translated.


345-345: LGTM! Placeholder indicators should not be translated.

Correctly marking the placeholder indicator "-" with notr="true" as these are UI placeholders that should remain consistent across languages.

Also applies to: 440-440, 664-664

src/qt/governancelist.cpp (1)

598-609: LGTM! Proper pluralization and clearer message construction.

The changes correctly implement:

  1. Qt's plural-aware formatting with %n for both success and failure counts
  2. Linear message assembly using += operator
  3. More explicit condition checking (nSuccessful > 0 instead of !message.isEmpty())
  4. Proper separation of translatable text ("Errors:") from non-translatable content (error details)

This enables proper translations for different plural forms across languages.

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.

1 participant