Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix translator comment for Restore Wallet QInputDialog #629

Merged
merged 1 commit into from
Jul 23, 2022

Conversation

w0xlt
Copy link
Contributor

@w0xlt w0xlt commented Jul 11, 2022

Fix translator comment for Restore Wallet QInputDialog, as suggested in #471 (comment).

This also changes the window title name from Restore Name to Restore Wallet as it seems clearer.

@hebasto hebasto changed the title gui: Fix translator comment for Restore Wallet QInputDialog Fix translator comment for Restore Wallet QInputDialog Jul 15, 2022
Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

LGTM ACK d7207df

  • The added translation comment clearly explains the purpose of the text for which it is added.

A small nit:
I would suggest mentioning the title's renaming in the commit message as well.

Edit: Address this comment. Thanks for notifying @hebasto!

@hebasto
Copy link
Member

hebasto commented Jul 19, 2022

@shaavan

LGTM d7207df

"LGTM" cannot be parsed by the merging script.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK d7207df, verified using make -C src translate.

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
@w0xlt
Copy link
Contributor Author

w0xlt commented Jul 21, 2022

#629 (comment) and #629 (review) were addressed in 643e27e.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

translator comments look good enough to me, maybe could do the following if you believe it's an improvement?

--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -430,9 +430,10 @@ void BitcoinGUI::createActions()
             if (backup_file.isEmpty()) return;
 
             bool wallet_name_ok;
-            //: Title of the Restore Wallet input dialog (where the wallet name is entered).
+            /*: Title of pop-up window shown when the user is attempting to
+                restore a wallet. */
             QString title = tr("Restore Wallet");
-            //: Label of the Restore Wallet input dialog (where the wallet name is entered).
+            //: Label of the input field where the name of the wallet is entered.
             QString label = tr("Wallet Name");
             QString wallet_name = QInputDialog::getText(this, title, label, QLineEdit::Normal, "", &wallet_name_ok);
             if (!wallet_name_ok || wallet_name.isEmpty()) return;

This also changes the window title name
from `Restore Name` to `Restore Wallet`.
@w0xlt
Copy link
Contributor Author

w0xlt commented Jul 23, 2022

#629 (review) addressed in 9d9a098

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

reACK 9d9a098

Changes since my last review:

  • The translator's comments have been updated.

I feel the updated translator comments are an improvement and better clarify the purpose of the title and label Qstrings.

@hebasto hebasto merged commit 194f6dc into bitcoin-core:master Jul 23, 2022
@w0xlt w0xlt deleted the 471-follow-up branch July 24, 2022 02:43
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 24, 2022
… `QInputDialog`

9d9a098 gui: Fix translator comment for Restore Wallet QInputDialog (w0xlt)

Pull request description:

  Fix translator comment for Restore Wallet `QInputDialog`, as suggested in bitcoin-core/gui#471 (comment).

  This also changes the window title name from `Restore Name` to `Restore Wallet` as it seems clearer.

ACKs for top commit:
  shaavan:
    reACK 9d9a098

Tree-SHA512: 02aec661839215ab1183e4e92fa131671daa986339373a87c0a0e2c5e79a46f362a8846f4a5f6d630a99884a7949031982d13352336bd3f0573625826406dde8
hebasto added a commit that referenced this pull request Sep 14, 2022
5f28fc8 qt: Cleanup translation comment (Hennadii Stepanov)

Pull request description:

  An unneeded character slipped in #629.

ACKs for top commit:
  jarolrod:
    ACK 5f28fc8
  jonatack:
    utACK 5f28fc8

Tree-SHA512: 210fb626e8035786cf6859160c60b2815c813e02908c75efc71a2c64d511edd6f81b2f67f1c98b29122b990260ebf663da445ea2d01b6268e3e046ada1ca5b6e
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 14, 2022
5f28fc8 qt: Cleanup translation comment (Hennadii Stepanov)

Pull request description:

  An unneeded character slipped in bitcoin-core/gui#629.

ACKs for top commit:
  jarolrod:
    ACK 5f28fc8
  jonatack:
    utACK 5f28fc8

Tree-SHA512: 210fb626e8035786cf6859160c60b2815c813e02908c75efc71a2c64d511edd6f81b2f67f1c98b29122b990260ebf663da445ea2d01b6268e3e046ada1ca5b6e
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants