docs: update README notes for /interfaces #208

pull RandyMcMillan wants to merge 2 commits into bitcoin-core:master from RandyMcMillan:interfaces-notes changing 2 files +25 −4
  1. RandyMcMillan commented at 7:24 pm on February 1, 2021: contributor
    • The contents of the folder have changed.
    • Added a reference to the src/node/ folder as a prompt for further reading.
  2. docs: update README notes for /interfaces e9f3b5906b
  3. hebasto commented at 8:26 pm on February 1, 2021: member
    @RandyMcMillan it seems this PR should be moved to the main repo https://github.com/bitcoin/bitcoin, no?
  4. RandyMcMillan commented at 9:18 pm on February 1, 2021: contributor

    Would you mind suggesting any improvements here? Since it is here? Then once ready - I can push it to bitcoin/bitcoin :)

    If it is good to go - I will push now.

  5. hebasto commented at 9:24 pm on February 1, 2021: member

    Would you mind suggesting any improvements here? Since it is here?

    tbh, I don’t see much value in these changes.

  6. in src/interfaces/README.md:39 in e9f3b5906b outdated
    37+* [`Init`](../init.h) — used by multiprocess code to access interfaces above on startup. Added in [#10102](https://github.com/bitcoin/bitcoin/pull/10102).
    38+
    39+
    40+---
    41 
    42 The interfaces above define boundaries between major components of bitcoin code (node, wallet, and gui), making it possible for them to run in different processes, and be tested, developed, and understood independently. These interfaces are not currently designed to be stable or to be used externally.
    


    jonatack commented at 9:30 pm on February 1, 2021:
    0The interfaces above define boundaries between major components of bitcoin code (node, wallet, and gui), making it possible for them to run in different processes and be tested, developed, and understood independently. These interfaces are not currently designed to be stable or to be used externally.
    
  7. jonatack commented at 9:32 pm on February 1, 2021: contributor
    Concept ACK, I find this added context interesting.
  8. in src/interfaces/README.md:1 in 4f86f39f74 outdated
    0@@ -1,17 +1,39 @@
    1-# Internal c++ interfaces
    2+# Internal c++ [`src/interfaces/`](../interfaces)
    


    jonatack commented at 10:47 pm on February 7, 2021:
    Might want to leave this as just “interfaces”, as you also discuss src/node
  9. in src/interfaces/README.md:14 in 4f86f39f74 outdated
    11 
    12 * [`Node`](node.h) — used by GUI to start & stop bitcoin node. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244).
    13+	- Refactored in [#20494](https://github.com/bitcoin/bitcoin/pull/20494/commits)
    14 
    15 * [`Wallet`](wallet.h) — used by GUI to access wallets. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244).
    16+	- Refactored in [#20494](https://github.com/bitcoin/bitcoin/pull/20494/commits)
    


    jonatack commented at 10:49 pm on February 7, 2021:
    Perhaps drop the /commits path in the URL and add this line in one place only, referring to chain/node/wallet.
  10. in src/interfaces/README.md:26 in 4f86f39f74 outdated
    25+
    26+The [`src/node/`](../node) directory is a new directory introduced in
    27+[#14978](https://github.com/bitcoin/bitcoin/pull/14978) and at the moment is
    28+sparsely populated. Eventually more substantial files like
    29+[`src/validation.cpp`](../validation.cpp) and
    30+[`src/txmempool.cpp`](../txmempool.cpp) may be moved there.
    


    jonatack commented at 10:51 pm on February 7, 2021:
    Might be good to link to a source for this last sentence.
  11. jonatack commented at 10:52 pm on February 7, 2021: contributor
    A few comments.
  12. in src/node/README.md:22 in 4f86f39f74 outdated
    18@@ -19,4 +19,4 @@ The [`src/node/`](./) directory is a new directory introduced in
    19 [#14978](https://github.com/bitcoin/bitcoin/pull/14978) and at the moment is
    20 sparsely populated. Eventually more substantial files like
    21 [`src/validation.cpp`](../validation.cpp) and
    22-[`src/txmempool.cpp`](../txmempool.cpp) might be moved there.
    23+[`src/txmempool.cpp`](../txmempool.cpp) may be moved there.
    


    jarolrod commented at 1:42 am on February 8, 2021:

    small NIT

    feel free to ignore this, don’t want this to get caught up on this word choice

    While may can be used in a scenario signaling possibility, it is primarily used in the context of signaling permission to do something. Considering the context, I believe that might is the correct choice here.


    jonatack commented at 9:03 pm on March 4, 2021:
    I prefer “may” and disagree with the comment that it is primarily about permission instead of possibiity; both are auxiliary definitions of the word and it’s clear that the context here is about possibility.

    jarolrod commented at 9:48 pm on March 4, 2021:
    @jonatack you’re right
  13. jarolrod commented at 1:43 am on February 8, 2021: member

    Concept ACK

    Thanks for documenting the refactors, small NIT on word choice.

  14. in src/interfaces/README.md:38 in 62b7a23098 outdated
    38+* [`Init`](../init.h) — used by multiprocess code to access interfaces above on startup. Added in [#10102](https://github.com/bitcoin/bitcoin/pull/10102).
    39+
    40+
    41+---
    42+
    43+The interfaces above define boundaries between major components of bitcoin code (node, wallet, and gui), making it possible for them to run in different processes and be tested, developed, and understood independently. These interfaces are not currently designed to be stable or to be used externally.
    


    jarolrod commented at 7:32 am on March 4, 2021:

    nit, I believe GUI is supposed to be capitalized

    0The interfaces above define boundaries between major components of bitcoin code (node, wallet, and GUI), making it possible for them to run in different processes and be tested, developed, and understood independently. These interfaces are not currently designed to be stable or to be used externally.
    
  15. jarolrod commented at 7:33 am on March 4, 2021: member
    small nit, @RandyMcMillan are you going to get back to this? This PR is a good addition to the documentation
  16. RandyMcMillan commented at 7:57 pm on March 4, 2021: contributor
    Yes - today
  17. RandyMcMillan marked this as a draft on Mar 4, 2021
  18. RandyMcMillan marked this as ready for review on Mar 4, 2021
  19. in src/interfaces/README.md:1 in d86cb0742b outdated
    0@@ -1,6 +1,6 @@
    1-# Internal c++ interfaces
    2+# Internal c++ 
    


    jarolrod commented at 9:50 pm on March 4, 2021:

    Linter shows leading whitespace at end of line

    0diff --git a/src/interfaces/README.md b/src/interfaces/README.md
    1@@ -1 +1 @@
    2+# Internal c++
    3^---- failure generated from test/lint/lint-whitespace.sh
    
  20. hebasto commented at 7:07 pm on March 6, 2021: member

    @RandyMcMillan

    Would you mind suggesting any improvements here? Since it is here? Then once ready - I can push it to bitcoin/bitcoin :)

    Many people, whose review could be valuable, could just miss this discussion due to the fact that the GUI repo has its own notifications (btw, that was one of the reasons to split Bitcoin Core repo – do not bother ppl with notifications they are not much interested in).

  21. hebasto added the label Doc on Mar 6, 2021
  22. Update src/interfaces/README.md
    Co-authored-by: Jon Atack <jon@atack.com>
    9165aa4389
  23. RandyMcMillan marked this as a draft on Mar 30, 2021
  24. RandyMcMillan closed this on Apr 20, 2021

  25. bitcoin-core locked this on Aug 16, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-23 09:20 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me