add client startup time as an entry to debug.log #1370
Conversation
|
I dont see any reason to make nClientStartupTime yet another global, throwing in one more call to GetTime() seems cleaner to me, but maybe I just hate globals. |
|
The global is used by the GUI Debug window and IS present in the current code. It only moved from clientmodel.cpp to util.cpp, to allow usage in the debug.log entry. |
|
Putting something in util.h is very different from qt/clientmodel.cpp, but Im probably just overly global-hating. |
|
Luke-Jr encouraged me to use a global ... seems like it's hard sometimes to get to the best approach. If no one thinks this is beneficial, just close the pull. |
|
I don't see why you need a global for this? You only use it once... |
|
I think its beneficial, and have no problem with it in general, my point was simply to remove the global definitions, and replace the single use in init.cpp with a call to GetTime(). |
|
I checked the GUI code and yes, I can safely remove the whole global ;). Will update tomorrow! |
| @@ -130,9 +129,9 @@ QString ClientModel::clientName() const | |||
| return QString::fromStdString(CLIENT_NAME); | |||
| } | |||
|
|
|||
| QDateTime ClientModel::formatClientStartupTime() const | |||
| QString ClientModel::formatGetTime() const | |||
There was a problem hiding this comment.
Doesn't Qt have it's own format time function? There's no need to put this in clientmodel as this requests nothing from the client.
There was a problem hiding this comment.
When removing the constant, I did not consider to remove this, will update.
There was a problem hiding this comment.
Ah I get it. I guess you want to store the time when the ClientModel is created. Better to add a QDateTime field to the structure, fill it in the constructor, then return that?
There was a problem hiding this comment.
I would say it's even sufficient to add the time once in the RPCConsole constructor, via ui->startupTime->setText(QDateTime::fromTime_t(GetTime()).toString());, or should that stay in RPCConsole::setClientModel()?
There was a problem hiding this comment.
IMO it should stay in ClientModel. Don't put such state in views, better not to rely on it that RPCConsole is created once at the beginning of the program and kept around forever. This may change later.
There was a problem hiding this comment.
I now only changed the code in ClientModel to return a QString, to not rely on RPCConsole object persistence.
There was a problem hiding this comment.
Yeah that's fine... though officially it'd be better to return a datetime as that leaves formatting to the view instead of the model... in this case this is the same as we do for the other datetimes. It's only a minor thing anyway.
…n debug.log differs by a few seconds from the one displayed in the Debug window) / make ClientModel::formatClientStartupTime() return a QString
|
Alright, updated and final now. |
|
ACK |
|
Yeah, while a global makes sense for what you were doing before, this one-time use doesn't need it ;) |
|
@luke-jr :-D Was that an ACK then ;)? |
add client startup time as an entry to debug.log
|
Looks good, but I only ACK stuff after I have actually tested it myself. ;) |
add client startup time as an entry to debug.log
* Use reference in range based loop * Test for TxAlreadyHave() before deciding whether to process What we were doing before was causing us to fall through and end up removing all the coins that were just put in cache if we processed a transation that was already in the commiQ or mempool. Futhermore we were marking it as a recentReject incorrectly.
Note: Logged time in debug.log differs by a few seconds from the one displayed in the Debug window because of different initialisation times.