Skip to content

Conversation

@fanquake
Copy link
Member

Deduplicate the makefile comments, in favour of doc/libraries.md. I think a single, more comprehensive source of truth is preferable. Diagrams are also useful. Came up in #26292 (comment).

This duplicates and is less explanatory than doc/libraries.md.
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplicate the makefile comments...

Concept ACK on that (especially, in the light of #25797).

if !ENABLE_WALLET
libbitcoin_node_a_SOURCES += dummywallet.cpp
endif
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fanquake, just curious why the trailing comment line here without any comment. Is this for automated documentation?

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #26298 (refactor: Move src/interfaces/*.cpp files to libbitcoin_common.a by ryanofsky)

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.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK af781bf, nice cleanups

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK af781bf, I have reviewed the code and it looks OK, I agree it can be merged.

@maflcko maflcko merged commit ba441d4 into bitcoin:master Oct 18, 2022
@fanquake fanquake deleted the lib_doc_consolidate branch October 18, 2022 13:01
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 23, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Oct 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants