clang-format all the things! #5387

1. 
   sipa commented at 4:47 PM on November 27, 2014: member

   Included is a script that will automatically format every file. Several files and directories are excluded for now, either because they come from a different project, because their coding style is significantly different already, or the result is just too terrible to see.

   Also note the trick used in bloom.cpp to avoid the ugly all-constructor-arguments-on-one-line that sometimes happens.

2. Add auto-formatter script a1261296e4
3. Reformat CBloomFilter::CBloomFilter comments 5ba376a9bc
4. Apply autoformat script 2037211a1a
5. sipa cross-referenced this on Nov 27, 2014 from issue Before 0.10 release cleanup source with clang-style-script by Diapolo
6. 
   sipa commented at 4:50 PM on November 27, 2014: member

   @Diapolo early christmas present.

7. 
   TheBlueMatt commented at 6:14 PM on November 27, 2014: contributor

   Another way to title this would have been "BREAK ALL THE MERGES!". Still, concept ACK, I'll check this next week...

8. 
   jonasschnelli commented at 7:12 PM on November 27, 2014: contributor

   Nice work. Idea for improvements:

   1. Could the script be parameterized to just clang-format the files which are in the current tracked by git?
   2. Could the script be parameterized to just clang-format the files of a specific commit?

   ACK on the script. Did not review the files.

9. 
   sipa commented at 11:43 PM on November 27, 2014: member

   1. Could the script be parameterized to just clang-format the files which are in the current tracked by git?

   Sounds like a good idea, but not a blocker imho.

   1. Could the script be parameterized to just clang-format the files of a specific commit?

   Yes, that would be interesting as an option. However, I think we want to get into a state where most of the code, most of the time, is mostly according to clang-format standards already, and re-applying the script (it takes 1.2s here) to the whole tree should be mostly a no-op anyway.

   On the other hand, if the resulting state is that there are always a few commits merged in that broke coding style, you don't want to independent pull requests to go clean those up automatically. Your suggestion is one way to avoid that. Another is to just periodically (and perhaps automatically) have a pull request created that applies coding style fixes.

10. 
    jonasschnelli commented at 7:17 AM on November 28, 2014: contributor

    Could the script be parameterized to just clang-format the files which are in the current tracked by git?

    Sounds like a good idea, but not a blocker imho.

    Yes. Totally non-blocking-this-pull idea.

11. laanwj added this to the milestone 0.10.0 on Nov 28, 2014
12. 
    sipa commented at 10:30 AM on November 28, 2014: member

    So I think a requirement here is that the formatting can be reproduced by others.

    I got my clang-format-3.5 from (apt sources line):

    deb http://llvm.org/apt/trusty/ llvm-toolchain-trusty-3.5 main
    

13. laanwj added the label Improvement on Nov 28, 2014
14. 
    laanwj commented at 11:28 AM on November 28, 2014: member

    Concluding from the discussion on IRC I think it would be better to postpone this until clang-format is more stable. Even different versions within 3.5 give different results - or at least: the clang-format-3.5 in Ubuntu 14.04 refuses to accept the input file completely, and in 3.6 some more things change (see the diff in my above post).

    To make this repeatable we would have to lock in one specific commit tag of clang/LLVM. That kind of defeats the advantages of being able to do this (and check this) automatcially.

15. 
    dexX7 commented at 12:42 PM on November 30, 2014: contributor

    I got my clang-format-3.5 from (apt sources line)

    It can also be extracted from the binary packages from http://llvm.org/releases/download.html and it is available as /bin/clang-format (e.g. clang+llvm-3.5.0-x86_64-linux-gnu.tar.xz -> ./clang+llvm-3.5.0-x86_64-linux-gnu/bin/clang-format).

    It worked for me on Ubuntu 14.04 after moving auto-format.sh into the root dir and replacing "clang-format-3.5" by the actual formatter file I have somewhere else.

16. 
    sipa commented at 1:10 PM on November 30, 2014: member

    @dexX7 Was the result identical?

17. 
    sipa commented at 1:10 PM on November 30, 2014: member

    @dexX7 Also, no need to move it. You can run it from the root. (i.e., ./contrib/devtools/auto-format.sh).

18. 
    dexX7 commented at 1:49 PM on November 30, 2014: contributor

    Yes, that seems to be the case. I reverted your last commit and run it here: https://github.com/dexX7/bitcoin/commit/d328f6e1b12600cdfef28d29da946a331d24dfa7

    May I ask why headers are excluded?

19. 
    sipa commented at 1:52 PM on November 30, 2014: member

    They're not?

    EDIT: Ugh, they seem to be.

20. laanwj added this to the milestone 0.11.0 on Dec 11, 2014
21. laanwj removed this from the milestone 0.10.0 on Dec 11, 2014
22. 
    laanwj commented at 2:51 PM on December 11, 2014: member

    Bumped to 0.11.

23. 
    Diapolo commented at 7:18 AM on December 12, 2014: none

    Why?

24. 
    laanwj commented at 10:12 AM on December 12, 2014: member

    @diapolo Please read the above discussion. Clang-format needs more time to stabilize.

25. 
    sipa commented at 4:36 AM on April 8, 2015: member

    Closing as highly outdated.

26. sipa closed this on Apr 8, 2015

    ---

27. jonasschnelli cross-referenced this on May 19, 2015 from issue Various PEP8 Fixes by super3
28. jonasschnelli cross-referenced this on Jun 6, 2015 from issue Transactions submitted by sendrawtransaction rpccall should be always allowed despite this new -relaypriority switch. by wangchun
29. 
    Diapolo commented at 2:51 PM on October 8, 2015: none

    Should be done finally...

30. bitcoin locked this on Sep 8, 2021

Contributors

sipaTheBlueMattjonasschnellilaanwjdexX7Diapolo

Milestone
0.11.0