Use `PACKAGE_NAME` in messages rather than hardcoding "Bitcoin Core" #26142

hebasto commented at 2:42 PM on September 20, 2022: member

Usually, we do not hardcode "Bitcoin Core" in the user-faced messages.

See:

bitcoin/bitcoin#18646
bitcoin/bitcoin#19282

Also grammar has been improved -- singular instead of plural.

jarolrod commented at 3:20 AM on September 21, 2022: member

This doesn't work as is <img width="162" alt="Screen Shot 2022-09-20 at 11 20 00 PM" src="https://user-images.githubusercontent.com/23396902/191407015-a0d903a5-265b-4ab8-b5af-96ae5ae584ee.png">

hebasto marked this as a draft on Sep 21, 2022

hebasto marked this as ready for review on Sep 21, 2022

hebasto commented at 11:00 AM on September 21, 2022: member

@jarolrod

This doesn't work as is

Thanks! Updated.

hebasto force-pushed on Sep 21, 2022

fanquake requested review from luke-jr on Sep 21, 2022

in src/init.cpp:1665 in d895eb254b outdated

1661 | @@ -1662,11 +1662,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1662 |          static_cast<uint16_t>(args.GetIntArg("-port", Params().GetDefaultPort()));
1663 |  
1664 |      const auto BadPortWarning = [](const char* prefix, uint16_t port) {
1665 | -        return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
1666 | -                           "thus it is unlikely that any Bitcoin Core peers connect to it. See "
1667 | +        return strprintf(_("%s requests to listen on port %u. This port is considered \"bad\" and "

luke-jr commented at 3:35 AM on September 22, 2022:

-bind is more of an action than a noun, so I think the original here is correct

hebasto commented at 10:10 PM on September 27, 2022:

Thanks. A commit dropped.

in src/init.cpp:1666 in d895eb254b outdated

1661 | @@ -1662,11 +1662,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1662 |          static_cast<uint16_t>(args.GetIntArg("-port", Params().GetDefaultPort()));
1663 |  
1664 |      const auto BadPortWarning = [](const char* prefix, uint16_t port) {
1665 | -        return strprintf(_("%s request to listen on port %u. This port is considered \"bad\" and "
1666 | -                           "thus it is unlikely that any Bitcoin Core peers connect to it. See "
1667 | +        return strprintf(_("%s requests to listen on port %u. This port is considered \"bad\" and "
1668 | +                           "thus it is unlikely that any %s peers connect to it. See "

luke-jr commented at 3:39 AM on September 22, 2022:

                           "thus it is unlikely that any peers will connect to it. See "

No reason to name the software specifically here?

jarolrod commented at 3:50 AM on September 22, 2022:

true

jarolrod commented at 9:42 PM on September 27, 2022:

On second thought, we are touching this, but it isn’t a part of the pr’s goal to address something like this; seems appropriate for a follow-up if someone wants to suggest this change.

hebasto commented at 10:11 PM on September 27, 2022:

No reason to name the software specifically here?

Updated.

in src/qt/forms/optionsdialog.ui:289 in d895eb254b outdated

 285 | @@ -286,7 +286,7 @@
 286 |              <item>
 287 |               <widget class="QLineEdit" name="externalSignerPath">
 288 |                <property name="toolTip">
 289 | -               <string>Full path to a Bitcoin Core compatible script (e.g. C:\Downloads\hwi.exe or /Users/you/Downloads/hwi.py). Beware: malware can steal your coins!</string>
 290 | +               <string>Full path to a %1 compatible script (e.g. C:\Downloads\hwi.exe or /Users/you/Downloads/hwi.py). Beware: malware can steal your coins!</string>

luke-jr commented at 3:40 AM on September 22, 2022:

ACK

in src/qt/optionsdialog.cpp:95 in d895eb254b outdated

  90 | @@ -91,7 +91,9 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) :
  91 |          ui->thirdPartyTxUrls->setVisible(false);
  92 |      }
  93 |  
  94 | -#ifndef ENABLE_EXTERNAL_SIGNER
  95 | +#ifdef ENABLE_EXTERNAL_SIGNER
  96 | +    ui->externalSignerPath->setToolTip(ui->externalSignerPath->toolTip().arg(PACKAGE_NAME));

luke-jr commented at 3:41 AM on September 22, 2022:

Might be nice if there was a BIP we could specify here...

DrahtBot commented at 3:15 AM on September 23, 2022: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

glozow added the label Refactoring on Sep 23, 2022

Use `PACKAGE_NAME` in messages rather than hardcoding "Bitcoin Core" b147322a7a

hebasto force-pushed on Sep 27, 2022

hebasto commented at 10:09 PM on September 27, 2022: member

Updated d895eb254b8fee30e13c190409f7b53a447c6416 -> b147322a7a387423164de5a7d91a33eacad51689 (pr26142.02 -> pr26142.03, diff):

addressed @luke-jr's comments

luke-jr approved

luke-jr commented at 2:58 AM on September 28, 2022: member

utACK

hebasto commented at 6:24 PM on October 18, 2022: member

Friendly ping @jarolrod.

jarolrod approved

jarolrod commented at 6:57 AM on October 19, 2022: member

ACK b147322a7a387423164de5a7d91a33eacad51689

No visual difference between pr and master, as expected.

pr	master
<img width="180" alt="Screen Shot 2022-10-19 at 2 39 44 AM" src="https://user-images.githubusercontent.com/23396902/196618670-1d6a0fea-fcae-4a22-b1e0-b518fbe981a9.png">	<img width="180" alt="Screen Shot 2022-10-19 at 2 55 41 AM" src="https://user-images.githubusercontent.com/23396902/196618778-1987f465-12b0-4716-8eef-d1c28f7fa721.png">

maflcko merged this on Oct 19, 2022

maflcko closed this on Oct 19, 2022

hebasto deleted the branch on Oct 19, 2022

sidhujag referenced this in commit 803371c207 on Oct 23, 2022

bitcoin locked this on Oct 19, 2023