Correct indentation #4207

pull rebroad wants to merge 3 commits into bitcoin:master from rebroad:CorrectIndentation changing 1 files +23 −23

rebroad commented at 11:54 AM on May 21, 2014: contributor

Same result as #4202 except now as three commits to help reduce confusion with the diffs.

Use meld or something more intelligent to view the difference please - github's diff is rather misleading.
Correct indentation 672ce36233
Add extra { and } to make more readable ae43be79de
Reduce lines needed 0b05cbd29c
BitcoinPullTester commented at 12:18 PM on May 21, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0b05cbd29ca8cb8ba9ded4d844f2ff0b4adba16e 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.
Diapolo commented at 1:27 PM on May 21, 2014: none

ACK on indentation fixes, but squash into one commit.
rebroad commented at 5:28 PM on May 21, 2014: contributor

#4202 is the one commit version - can someone re-open it perhaps?
gmaxwell commented at 7:30 PM on May 21, 2014: contributor

Agreed. Formatting changes should be just that— formatting changes. Objdumps on the stripped binaries should not change if at all avoidable. Review is hard enough without having to sort out functional changes vs visual noise.

in src/main.cpp:None in 0b05cbd29c

3438 | -                    map<CInv, CDataStream>::iterator mi = mapRelay.find(inv);
3439 | -                    if (mi != mapRelay.end()) {
3440 | -                        pfrom->PushMessage(inv.GetCommand(), (*mi).second);
3441 | -                        pushed = true;
3442 | +            } else {
3443 | +                if (inv.IsKnownType()) {

rebroad commented at 4:50 AM on May 22, 2014:

Are you saying putting { } around code is a code change? I can remove the commit that added the { and }, but given that there are many lines between the two, I thought the standard was to include them to make it more readable - at least, this has been the standard so far.

in src/main.cpp:None in 0b05cbd29c
```
3469 | +                            pfrom->PushMessage("tx", ss);
3470 | +                            pushed = true;
3471 | +                        }
3472 | +                    }
3473 | +                    if (!pushed) {
3474 | +                        vNotFound.push_back(inv);
```
rebroad commented at 4:51 AM on May 22, 2014:

It's not a convention, it's a requirement of the C++ language to put braces if there is more than one statement. It's optional when there is only one statement, and considered good practice when it's a statement that's quite long, as is in this case (i.e. a long if statement).

gmaxwell commented at 4:54 AM on May 22, 2014:

Please read drak's comment again.
laanwj commented at 8:26 AM on May 22, 2014: member

Closing this again. I am not going to merge this. As I already commented in #4202, trivial code changes that turn to to be controversial are not a good use of our review and testing time. Please don't keep posting this.
laanwj closed this on May 22, 2014
rebroad deleted the branch on Aug 7, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#4202 Correct indentation