net: Remove un-actionable TODO #18996

1. 
   MarcoFalke commented at 2:41 pm on May 17, 2020: member

   The first commit removes a TODO that is infeasible to solve. Currently, most (de)serializable classes in Bitcoin Core have public members. For example CMessageHeader, FlatFilePos, CBlock, CTransaction, CCoin, …

   So either this TODO comment should apply to all classes or to none. Fix that discrepancy by removing it from the source code for now. If deemed important, the TODO can be discussed in a brainstorming issue later.

   Also run clang format on the header file in a new commit. Happy to drop this commit if it is too controversial, but I think it is trivial to review and makes the workflow of developers using clang-format-diff easier.

2. net: Remove un-actionable TODO facdeea2b2
3. net: Run clang-format on protocol.h

   Can be reviewed with the git diff flags
   -U0 --ignore-all-space --word-diff-regex=.

   fabea6d404
4. MarcoFalke added the label P2P on May 17, 2020
5. MarcoFalke added the label Refactoring on May 17, 2020
6. 
   practicalswift commented at 3:30 pm on May 17, 2020: contributor

   ACK fabea6d404571d046365f4f083da3569d2cbf4f7

   This is probably the only repo where a clang-format-diff.py runs the risk of be deemed “too controversial” :)

7. 
   DrahtBot commented at 9:13 pm on May 17, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #18876 ([WIP] Serve BIP 157 compact filters by jnewbery)
   • #18242 (Add BIP324 encrypted p2p transport de-/serializer (only used in tests) by jonasschnelli)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

8. hebasto approved
9. 
   hebasto commented at 6:08 am on May 18, 2020: member

   ACK fabea6d404571d046365f4f083da3569d2cbf4f7, agree with both changes: removing TODO and applying the clang-format-diff.py.
10. 
    naumenkogs commented at 3:48 pm on May 18, 2020: member

    ACK fabea6d. Not sure why that TODO was there in the first place, but Marco’s justification seems correct.

    Formatting seems correct.

11. 
    MarcoFalke commented at 4:25 pm on May 18, 2020: member

    Not sure why that TODO was there in the first place

    The TODO was added in one of the first commits. I think it made sense back when it was added.

12. MarcoFalke merged this on May 20, 2020
13. MarcoFalke closed this on May 20, 2020

    ---

14. MarcoFalke deleted the branch on May 20, 2020
15. sidhujag referenced this in commit 6b511cc085 on May 20, 2020
16. Fabcien referenced this in commit d9bec55622 on Feb 3, 2021
17. DrahtBot locked this on Feb 15, 2022

Contributors
MarcoFalke practicalswift DrahtBot hebasto naumenkogs

Labels
Refactoring P2P