Add address relay/processed/rate-limited fields to peer details by jonatack · Pull Request #526 · bitcoin-core/gui

jonatack · 2022-01-20T23:10:41Z

This pull adds the following address fields in rpc getpeerinfo and cli -netinfo to the gui peers details:

Address Relay (Yes/No)
Addresses Processed (integer)
Addresses Rate-Limited (integer)

and uses the additional horizontal space to display "Last Transaction" (instead of "Last Tx").

DrahtBot · 2022-01-21T07:29:46Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#505 (RPCConsole: add hidePeersDetail() button and functionality by RandyMcMillan)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

w0xlt

tACK 5efd391 on Ubuntu 21.10 (Qt 5.15.2).

hebasto · 2022-01-24T08:21:55Z

Concept ACK, the added fields correspond to the getpeerinfo RPC output.

src/qt/forms/debugwindow.ui

jonatack · 2022-01-24T10:01:39Z

Thanks @w0xlt and @hebasto, updated.

git diff a00bc71 9fbd1bb

diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui
index 174954087..219680102 100644
--- a/src/qt/forms/debugwindow.ui
+++ b/src/qt/forms/debugwindow.ui
@@ -1355,7 +1355,7 @@
                <item row="13" column="0">
                 <widget class="QLabel" name="peerLastTxLabel">
                  <property name="toolTip">
-                  <string>Elapsed time since a novel transaction accepted into our mempool was received from this peer.</string>
+                  <string extracomment="Tooltip text for the Last Transaction field in the peer details area.">Elapsed time since a novel transaction accepted into our mempool was received from this peer.</string>
                  </property>
                  <property name="text">
                   <string>Last Transaction</string>
@@ -1594,7 +1594,7 @@
                <item row="23" column="0">
                 <widget class="QLabel" name="peerAddrRelayEnabledLabel">
                  <property name="toolTip">
-                  <string>Whether we relay addresses to this peer.</string>
+                  <string extracomment="Tooltip text for the Address Relay field in the peer details area.">Whether we relay addresses to this peer.</string>
                  </property>
                  <property name="text">
                   <string>Address Relay</string>
@@ -1620,7 +1620,7 @@
                <item row="24" column="0">
                 <widget class="QLabel" name="peerAddrProcessedLabel">
                  <property name="toolTip">
-                  <string>Total number of addresses processed, excluding those dropped due to rate limiting.</string>
+                  <string extracomment="Tooltip text for the Addresses Processed field in the peer details area.">Total number of addresses processed, excluding those dropped due to rate-limiting.</string>
                  </property>
                  <property name="text">
                   <string>Addresses Processed</string>
@@ -1646,7 +1646,7 @@
                <item row="25" column="0">
                 <widget class="QLabel" name="peerAddrRateLimitedLabel">
                  <property name="toolTip">
-                  <string>Total number of addresses dropped due to rate limiting.</string>
+                  <string extracomment="Tooltip text for the Addresses Rate-Limited field in the peer details area.">Total number of addresses dropped due to rate-limiting.</string>
                  </property>
                  <property name="text">
                   <string>Addresses Rate-Limited</string>

hebasto

ACK 9fbd1bb, tested on Ubuntu 21.10 (Qt 5.15.2).

w0xlt

reACK 9fbd1bb

hebasto

Cross-posted feedback from French translation team.

hebasto · 2022-02-17T08:19:09Z

src/qt/forms/debugwindow.ui

+                  <string extracomment="Tooltip text for the Addresses Processed field in the peer details area.">Total number of addresses processed, excluding those dropped due to rate-limiting.</string>
+                 </property>
+                 <property name="text">
+                  <string>Addresses Processed</string>


A feedback from a Transifex translator:

Rather: "Processed Addresses"

If "processed" was an adjective, then I would agree with the feedback, i.e. "red car" and not "car red".

However, in this case it is a verb and this is fine as-is, i.e. "cars sold" vs "sold cars".

Hi @jonatack The lack of string context in Transifex is a real problem for this project, as proper context (dev notes and/or screenshots) are essential to achieve quality translations.
How is this string used, is this a title, a tooltip, a notification? Do we actually mean “Addresses are processed”?

@AO-LocLab it is the field name in the peer details area, as displayed in the screenshot in the PR description. There is also a tooltip saying this, and its explanatory description, 3 lines higher at line 1623. There is one of these for each added field. I don't know if they are visible in transifex.

<string extracomment="Tooltip text for the Addresses Processed field in the peer details area."> Total number of addresses processed, excluding those dropped due to rate-limiting.</string>

I don't know if they are visible in transifex.

Although, this one has no developer notes:

Should an extracomment be added, or something else?

Actually, extracomments in *.ui files are Qt translator comments which are presented on Transifex as "developer notes".

Should an extracomment be added...?

That is the plan :)

Ok! Will propose adding extracomments to the titles, then (if I understand correctly) and maybe to the tooltips without them.

Ok! Will propose adding extracomments to the titles, then (if I understand correctly) and maybe to the tooltips without them.

Thank you!

Started adding and improving the dev notes (and tooltips) in #554.

src/qt/forms/debugwindow.ui

…s tab address fields 4d2b503 gui: improve "Addresses Rate-Limited" translator comments and tooltip in peers tab (Jon Atack) 81ef1f7 gui: improve "Addresses Processed" translator comments and tooltip in peers tab (Jon Atack) 77f24aa gui: improve "Address Relay" translator comments and tooltip in peers tab (Jon Atack) Pull request description: Per translator feedback in this thread: #526 (comment) *"The lack of string context in Transifex is a real problem for this project, as proper context (dev notes and/or screenshots) are essential to achieve quality translations."* This pull adds developer notes for transifex translators via `extracomment` tags, and it improves the existing ones and their tooltips with more context, clarity and completeness for the following peer tab fields as a follow-up to #526: - address relay - addresses processed - addressed rate-limited It looks like only six lines of diff, but they are loooong lines. If this is the right direction, the same can be done for other fields in follow-ups. ACKs for top commit: jarolrod: re-ACK [4d2b503](4d2b503) hebasto: ACK 4d2b503, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: a185f46a66375a5fd6854640745b7d1d00740cf7be58db03256f44d71acc351e1770de137cb3bc9c1f0ea3cabd7cfa1cb1ccb87ec0df222680924ca3dab6c8bf

…ooltips for peers tab address fields 4d2b503 gui: improve "Addresses Rate-Limited" translator comments and tooltip in peers tab (Jon Atack) 81ef1f7 gui: improve "Addresses Processed" translator comments and tooltip in peers tab (Jon Atack) 77f24aa gui: improve "Address Relay" translator comments and tooltip in peers tab (Jon Atack) Pull request description: Per translator feedback in this thread: bitcoin-core/gui#526 (comment) *"The lack of string context in Transifex is a real problem for this project, as proper context (dev notes and/or screenshots) are essential to achieve quality translations."* This pull adds developer notes for transifex translators via `extracomment` tags, and it improves the existing ones and their tooltips with more context, clarity and completeness for the following peer tab fields as a follow-up to bitcoin-core/gui#526: - address relay - addresses processed - addressed rate-limited It looks like only six lines of diff, but they are loooong lines. If this is the right direction, the same can be done for other fields in follow-ups. ACKs for top commit: jarolrod: re-ACK [4d2b503](bitcoin-core/gui@4d2b503) hebasto: ACK 4d2b503, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: a185f46a66375a5fd6854640745b7d1d00740cf7be58db03256f44d71acc351e1770de137cb3bc9c1f0ea3cabd7cfa1cb1ccb87ec0df222680924ca3dab6c8bf

DrahtBot mentioned this pull request Jan 21, 2022

Hide peers details #505

Merged

w0xlt approved these changes Jan 23, 2022

View reviewed changes

hebasto added the Feature label Jan 24, 2022

hebasto reviewed Jan 24, 2022

View reviewed changes

src/qt/forms/debugwindow.ui Outdated Show resolved Hide resolved

jonatack added 4 commits January 24, 2022 10:44

gui: add "Address Relay" (m_addr_relay_enabled) to peer details

a465a66

gui: add "Addresses Processed" (m_addr_processed) to peer details

19623d3

gui: add "Addresses Rate-Limited" (m_addr_rate_limited) to peer details

6cd132d

gui: use available space to display "Last Transaction" in peer details

9fbd1bb

jonatack force-pushed the add-addr-fields-to-peer-details branch from a00bc71 to 9fbd1bb Compare January 24, 2022 09:57

hebasto approved these changes Jan 24, 2022

View reviewed changes

hebasto added this to the 23.0 milestone Jan 26, 2022

w0xlt approved these changes Jan 28, 2022

View reviewed changes

hebasto merged commit 5b4b8f7 into bitcoin-core:master Jan 28, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 29, 2022

Merge bitcoin-core/gui#526: Add address relay/processed/rate-limited …

8bdd318

hebasto reviewed Feb 17, 2022

View reviewed changes

jonatack deleted the add-addr-fields-to-peer-details branch February 17, 2022 09:11

jonatack mentioned this pull request Feb 20, 2022

Add and improve translator comments and tooltips for peers tab address fields #554

Merged

bitcoin-core locked and limited conversation to collaborators Feb 20, 2023

Comments

Conversation

jonatack commented Jan 20, 2022

Uh oh!

DrahtBot commented Jan 21, 2022

Conflicts

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 24, 2022

Uh oh!

Uh oh!

jonatack commented Jan 24, 2022

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

w0xlt left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatack Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

AO-LocLab Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

jonatack Feb 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 20, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 20, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatack Feb 20, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 20, 2022

Choose a reason for hiding this comment

Uh oh!

jonatack Feb 20, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hebasto Feb 17, 2022 •

edited

Loading

jonatack Feb 20, 2022 •

edited

Loading

hebasto Feb 20, 2022 •

edited

Loading