small update for BitcoinGUI::setNumBlocks() function #1594

Diapolo commented at 7:04 AM on July 13, 2012: none

re-work code parts that check if we have a Statusbar warning
use same variable names in the header

laanwj commented at 9:09 AM on July 14, 2012: member

I don't think we should be making this many changes just for the testnet.

The testnet is just for testing, and it is supposed to behave just like the mainnet (at a much lower level than the UI), so there should be as little special casing as possible.

Diapolo commented at 11:26 AM on July 14, 2012: none

@laanwj Alright, but then testnet3 will not catch-up as long as testnet2 our node connects to them.

Anything else in here that you think is merge-worthy? I'm fine with removing the testnet3 check but think the other small things are nice to have.

in src/qt/bitcoingui.cpp:None in 44423e1284 outdated

 487 | @@ -488,7 +488,7 @@ void BitcoinGUI::setNumConnections(int count)
 488 |      labelConnectionsIcon->setToolTip(tr("%n active connection(s) to Bitcoin network", "", count));
 489 |  }
 490 |  
 491 | -void BitcoinGUI::setNumBlocks(int count, int nTotalBlocks)
 492 | +void BitcoinGUI::setNumBlocks(int nBlocks, int nBlocksOfPeers)

laanwj commented at 6:54 AM on July 17, 2012:

Please don't use the ugly nXXX variable names here, thank you :-)

in src/qt/bitcoingui.cpp:None in 44423e1284 outdated

 510 | -        int nRemainingBlocks = nTotalBlocks - count;
 511 | -        float nPercentageDone = count / (nTotalBlocks * 0.01f);
 512 | +        float nPercentageDone = nBlocks / (nBlocksOfPeers * 0.01f);
 513 |  
 514 | -        if (clientModel->getStatusBarWarnings() == "")
 515 | +        if (strStatusBarWarnings.isEmpty())

laanwj commented at 7:27 AM on July 17, 2012:

Agreed, using isEmpty is better, and caching the status bar warnings is also better than requesting them twice.

laanwj commented at 7:28 AM on July 17, 2012: member

Yes, apart from the nXXX and strXXX variable convention that you're trying to sneak in :p, the other changes are OK with me.

Diapolo commented at 8:01 AM on July 17, 2012: none

I'll update this to be mergable ... you "catched" me again with that var names, heh ;)? But that's fine.

small update for BitcoinGUI::setNumBlocks() function

- re-work code parts that check if we have a Statusbar warning
- use same variable names in the header

ce2f66d9bf

Diapolo commented at 12:57 PM on July 17, 2012: none

Updated to "only" be a small function update ;). I guess if we don't plan to push a fix for #1588 I'll just close it.

burger2 commented at 7:14 PM on July 18, 2012: contributor

It would be nice to fix the testnet3 bug. But it might be fixed automatically when everyone is running 0.7.x.

Diapolo commented at 7:17 PM on July 18, 2012: none

@burger2 Yes, when more nodes are running 0.7.x everything will be fine again.

laanwj commented at 7:54 PM on July 18, 2012: member

If it ought to be fixed, it is on a deeper level than the ui, and not by special casing the testnet. Remember that the testnet is there to test how the client behaves in circumstances that are possible but unlikely on the main net. Special casing for testnet could effectively cover up problems.

Diapolo commented at 8:55 PM on July 19, 2012: none

@laanwj I had a talk with sipa about the underlying issue a few days ago and you are right of course. That's why the testnet3 casing is gone from this pull ;).

Diapolo commented at 8:52 AM on July 26, 2012: none

As this pull is now unrelated to testnet3, anything more I can do here?

laanwj referenced this in commit c1aed4eff4 on Jul 27, 2012

laanwj merged this on Jul 27, 2012

laanwj closed this on Jul 27, 2012

suprnurd referenced this in commit cd262bf641 on Dec 5, 2017

lateminer referenced this in commit e001ddf106 on May 6, 2020

DrahtBot locked this on Sep 8, 2021