init.cpp: log fee estimates filename on error #4428

Diapolo commented at 12:45 PM on June 27, 2014: none

small changes to Shutdown(), buffer __func__, which is now used in all LogPrintf() calls and format for better readability
order using namespace alpabetically

in src/init.cpp:None in 6c19908202 outdated

 995 | @@ -989,6 +996,8 @@ bool AppInit2(boost::thread_group& threadGroup)
 996 |      CAutoFile est_filein = CAutoFile(fopen(est_path.string().c_str(), "rb"), SER_DISK, CLIENT_VERSION);
 997 |      if (est_filein)
 998 |          mempool.ReadFeeEstimates(est_filein);
 999 | +    else
1000 | +        LogPrintf("%s: Failed to read fee estimates from %s\n", __func__, est_path.string());

gavinandresen commented at 2:10 PM on June 30, 2014:

NAK on this change.

The file will not exist the first time you run with the fee estimation code, and that is perfectly OK and expected.

Things that are Perfectly OK and expected should not be logged; that will just make people ask us why they are getting a "failed" in their debug.log.

Diapolo commented at 2:35 PM on June 30, 2014:

Understood and reverted, thanks for looking.

laanwj commented at 6:42 AM on July 1, 2014: member

This contains too many non-related changes. Please have some focus in commits, it makes it easier to review and breaks less other's work. And as I said, no whitespace changes please.

init.cpp: log fee estimates filename on error

- small changes to Shutdown(), buffer __func__, which is now used in
  all LogPrintf() calls and format for better readability
- order using namespace alpabetically

00d1980b8f

in src/init.cpp:None in 5921f28abf outdated

 317 | @@ -312,6 +318,7 @@ std::string HelpMessage(HelpMessageMode mode)
 318 |  
 319 |      strUsage += "\n" + _("Node relay options:") + "\n";
 320 |      strUsage += "  -datacarrier           " + _("Relay and mine data carrier transactions (default: 1)") + "\n";
 321 | +

Diapolo commented at 7:28 AM on July 1, 2014:

I want to argue, THIS new-line is there for every other block of options that are grouped :-D.

laanwj commented at 7:43 AM on July 1, 2014:

Fair enough, remove all whitespace changes except this one then.

Diapolo commented at 7:54 AM on July 1, 2014:

Removed the __func__ changes, so this is the only empty new-line and I remove no emtpy new-line. Sorry I'm a tad frustrated that testing of the larger pulls is a little slow and that coding-style stuff is just low-/no-priority.

laanwj commented at 8:00 AM on July 1, 2014:

I understand that, but I hope you also understand that pulls that fix issues or add important functionality rank a lot higher. We don't want to break those unnecessarily, it's hard enough as it is.

Anyhow: if you want your commits to be merged quickly try to fix bugs!

Diapolo commented at 6:16 AM on July 2, 2014:

Changes you didn't want are gone now.

in src/init.cpp:None in 5921f28abf outdated

 143 | @@ -141,9 +144,12 @@ void Shutdown()
 144 |              pblocktree->Flush();
 145 |          if (pcoinsTip)
 146 |              pcoinsTip->Flush();
 147 | -        delete pcoinsTip; pcoinsTip = NULL;

Diapolo commented at 7:29 AM on July 1, 2014:

Easy to read or to catch ones attention? IMHO no.

in src/init.cpp:None in 5921f28abf outdated

 107 | @@ -108,10 +108,13 @@ static CCoinsViewDB *pcoinsdbview;
 108 |  
 109 |  void Shutdown()
 110 |  {
 111 | -    LogPrintf("Shutdown : In progress...\n");
 112 | +    string strFunc(__func__);

laanwj commented at 7:33 AM on July 1, 2014:

Please don't assign __func__ to a variable here. __func__ is already a hidden variable added to every function by the compiler but just using it as-is makes it easier to pick out.

Diapolo commented at 7:38 AM on July 1, 2014:

Thanks for this valuable hint :).

in src/init.cpp:None in bad33bbcbd outdated

1174 | @@ -1169,7 +1175,7 @@ bool AppInit2(boost::thread_group& threadGroup)
1175 |  
1176 |      //// debug print
1177 |      LogPrintf("mapBlockIndex.size() = %u\n",   mapBlockIndex.size());
1178 | -    LogPrintf("nBestHeight = %d\n",                   chainActive.Height());

laanwj commented at 8:01 AM on July 1, 2014:

This one is pointless as well.

BitcoinPullTester commented at 7:44 AM on July 9, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4428_00d1980b8fb8b834cb729b213834dfb38cb01bbf/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj merged this on Jul 9, 2014

laanwj closed this on Jul 9, 2014

laanwj referenced this in commit e28c1ae75d on Jul 9, 2014

Diapolo deleted the branch on Jul 15, 2014

MarcoFalke locked this on Sep 8, 2021