name_pending Qt GUI #406
name_pending Qt GUI #406
Conversation
JeremyRand
force-pushed
the
qt-name-pending
branch
from
a1877ed
to
54eda7c
Compare
|
@randy-waterhouse You may be interested in reviewing this. |
JeremyRand
force-pushed
the
qt-name-pending
branch
from
54eda7c
to
ae2201c
Compare
JeremyRand
force-pushed
the
qt-name-pending
branch
2 times, most recently
from
1b9fd38
to
18fd8fd
Compare
|
@domob1812 This PR is ready for review. |
src/qt/nametablemodel.cpp
Outdated
| vNamesO[name] = NameTableEntry(name, data, height, expiresIn, "confirmed"); | ||
|
|
||
| bool isMine = find_value ( v, "ismine").get_bool(); | ||
| bool isExpired = find_value ( v, "expired").get_bool(); |
There was a problem hiding this comment.
Nit: Please make them const. Also the whitespace is again weird, but I see it matches the lines above.
src/qt/nametablemodel.cpp
Outdated
| std::string data = maybeData.get_str(); | ||
|
|
||
| bool isMine = find_value ( v, "ismine").get_bool(); | ||
| std::string op = find_value ( v, "op").get_str(); |
There was a problem hiding this comment.
All of the variables above can be const, right?
src/qt/nametablemodel.cpp
Outdated
| bool isMine = find_value ( v, "ismine").get_bool(); | ||
| std::string op = find_value ( v, "op").get_str(); | ||
|
|
||
| auto confirmedEntryExists = vNamesO.count(name); |
There was a problem hiding this comment.
Here I would suggest to explicitly use const bool as the type. This makes the intent of the code more explicit, is not any longer, and in fact explicitly makes the value bool instead of an integer.
src/qt/nametablemodel.cpp
Outdated
| int expiresIn = 0; | ||
| std::string status; | ||
|
|
||
| if (!confirmedEntryExists) |
There was a problem hiding this comment.
Optional nit: I think it helps code readability in general if negative conditions are avoided as much as possible. In this case, you have both an "if" and an "else" clause. So I would suggest to swap the two, so that the if is for the positive confirmedEntryExists.
src/qt/nametablemodel.cpp
Outdated
| status = NameTableEntry::NAME_STATUS_INCOMING_TRANSFER_PENDING; | ||
| } | ||
| } | ||
| else if (confirmedStatus == "Confirmed") |
There was a problem hiding this comment.
Is there a reason why we have constants for all the status strings but not "Confirmed"?
|
@domob1812 Feedback addressed; feel free to review again. Let me know if/when it's approved and I'll squash before you merge it. |
|
@domob1812 Any chance you could review this? |
|
ACK 769090e. Please squash and then I'll merge the PR. |
JeremyRand
force-pushed
the
qt-name-pending
branch
from
769090e
to
142c109
Compare
|
@domob1812 Squashed. (Left the name_list and name_pending components as distinct commits since each is atomic.) |
|
Merged, thanks! |
This PR adds the
name_pendingGUI, and also adds the GUI for theismineandexpiredfields inname_list.