Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[build] Actually remove ENABLE_WALLET #14208

Merged
merged 1 commit into from Sep 13, 2018

Conversation

Projects
None yet
9 participants
@jnewbery
Copy link
Member

commented Sep 12, 2018

Adds a couple of redefinitions to dummywallet.cpp.

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

Builds on #14204. Review that first.

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

utACK abdba5f59e348c7093bee2639ef6c2398e8d2f6c
(really closes #7965)

src/interfaces/node.cpp Outdated
@@ -221,15 +219,15 @@ class NodeImpl : public Node
}
std::vector<std::unique_ptr<Wallet>> getWallets() override
{
#ifdef ENABLE_WALLET
if (!g_wallet_init_interface.HasWalletSupport()) {
throw std::logic_error("Node::getWallets() called in non-wallet build.");

This comment has been minimized.

Copy link
@ryanofsky

ryanofsky Sep 12, 2018

Contributor

Can you drop g_wallet_init_interface references and throws in this file? They seem redundant if MakeWallet and GetWallets functions can throw now.

This comment has been minimized.

Copy link
@jnewbery

jnewbery Sep 12, 2018

Author Member

Done!

@jnewbery jnewbery force-pushed the jnewbery:actually_remove_enabled_wallet branch Sep 12, 2018

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2018

#14204 is merged. I've rebased on master and addresed @ryanofsky's feedback: #14208 (comment)

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

utACK 3efef4b225308954b23a30c74d7781ba87a51b76

@scravy

scravy approved these changes Sep 12, 2018

Copy link
Contributor

left a comment

utACK 3efef4b225308954b23a30c74d7781ba87a51b76

@ryanofsky
Copy link
Contributor

left a comment

utACK 3efef4b225308954b23a30c74d7781ba87a51b76

src/interfaces/node.cpp Outdated

#include <atomic>
#include <boost/thread/thread.hpp>
#include <univalue.h>

class CWallet;
class Wallet;

This comment has been minimized.

Copy link
@practicalswift

practicalswift Sep 12, 2018

Member
2018-09-12 23:23:50 clang-tidy(pr=14208): interfaces/node.cpp:41:7: warning: declaration 'Wallet' is never referenced, but a declaration with the same name found in another namespace 'interfaces' [bugprone-forward-declaration-namespace]
2018-09-12 23:23:50 clang-tidy(pr=14208): interfaces/node.cpp:41:7: warning: no definition found for 'Wallet', but a definition with the same name 'Wallet' found in another namespace 'interfaces' [bugprone-forward-declaration-namespace]

This comment has been minimized.

Copy link
@promag

promag Sep 13, 2018

Member

Yap, should be fixed.

This comment has been minimized.

Copy link
@jnewbery

jnewbery Sep 13, 2018

Author Member

fixed

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2018

Note to reviewers: This pull request conflicts with the following ones:
  • #10973 (Refactor: separate wallet from node 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.

[build] remove #ifdef ENABLE_WALLET from interfaces/node
Removes the final #ifdef ENABLE_WALLET from libbitcoin_server by calling
g_wallet_init_interface.HasWalletSupport(), and redifining GetWallets()
and MakeWallet() in dummywallet.cpp.

@jnewbery jnewbery force-pushed the jnewbery:actually_remove_enabled_wallet branch to e4ef4b4 Sep 13, 2018

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

I've addressed the comment in #14208 (comment) and force-pushed.

@promag

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

utACK e4ef4b4.

@laanwj laanwj merged commit e4ef4b4 into bitcoin:master Sep 13, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

laanwj added a commit that referenced this pull request Sep 13, 2018

Merge #14208: [build] Actually remove ENABLE_WALLET
e4ef4b4 [build] remove #ifdef ENABLE_WALLET from interfaces/node (John Newbery)

Pull request description:

  Adds a couple of redefinitions to dummywallet.cpp.

Tree-SHA512: d226bcccc46d089eac88beb54c31f6f18817682994b371f9793a5d28bec5d60dbdffacc8fc281807e25cc7f89da23e1f8f36fd99d12f8a40f77a972840e8c1b4

@laanwj laanwj referenced this pull request Sep 13, 2018

Closed

Remaining instances of ENABLE_WALLET in `libbitcoin_server.a` #7965

14 of 14 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.