Skip to content

added configuration for network, default mainnet #34

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

krtschmr
Copy link

@krtschmr krtschmr commented Feb 6, 2019

Based on my Issue #33 the following pull request

adds ability to set the network globally

MoneyTree.configure do |config|
  config.network = :testnet3
end

default will be :bitcoin, as every method had it as a default argument aswell

tests are all passing. nothing was changed, only added.

@coveralls
Copy link

coveralls commented Feb 6, 2019

Coverage Status

Coverage decreased (-0.2%) to 99.139% when pulling 52d4b38 on krtschmr:master into 2686577 on GemHQ:master.

@krtschmr
Copy link
Author

krtschmr commented May 8, 2019

Whats up in here?

@krtschmr
Copy link
Author

i'm also vouching to change the default network up to :testnet3, but this might break applications so that should come in future.

i wonder why nobody watched / edited / reviewed this merge request. it's super useful.

@krtschmr
Copy link
Author

krtschmr commented Aug 2, 2019

i really wonder, why this is such a dead gem. my pullrequest is very very useful and already working in production for a bitcoin acceptance platform.

@krtschmr
Copy link
Author

RIP. so sad to see. open source sometimes sucks.
community should be able to claim a repository for the sake of development

@q9f
Copy link
Collaborator

q9f commented Dec 3, 2021

Sorry for the late response. I'm happy to accept this but would love to see some tests for various network configs.

@bdescamps
Copy link

Hi, what do you think of completely remove the network hash argument on each method ( and only keep the network through MoneyTree instance) ? We lose compatibility with older version but i think it will be more readable.

@krtschmr
Copy link
Author

@bdescamps so you would like to read always from within the config? Would that still be threadsafe?

I want to have multiple instances running for each (test)net available. If they all read from one static config, than that would not work?

@bdescamps
Copy link

bdescamps commented Nov 16, 2022

To me it's threadsafe because every instance will take his own MoneyTree.configure do |config| . By removing the network override we avoid ambigous behavior ( ex MoneyTree.configure set to bitcoin and a method network argument set to testnet. I think we should see that like a database connector ( ActiveRecord by example). There is one entrypoint to set the environment ( config file) and every time we use the database connector it's recover the instance with environment.
In any case, this is a great feature, thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants